Closed Bug 14827 Opened 25 years ago Closed 25 years ago

FRAME and IFRAME attribute scrolling=no is ignored

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect, P2)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: gemma, Assigned: pollmann)

References

()

Details

(Keywords: testcase, Whiteboard: [PDT+] (scrolling="no" shouldn't be ignored))

Attachments

(1 file)

for frames with scrolling="no" still have the scroll bar visible.
Assignee: karnaze → pollmann
Eric, this must be a regression.
Assignee: pollmann → evaughan
If I had to guess, I'd say this regression was caused by evaughan's recent change to nsHTMLContentSink::StartLayout(). He added this line: 2323 evaughan 3.232 // initially show the scrollbars. We need to do this be cause ano the scrollbars. -EDV 2325 mWebShell->SetScrolling(-1, PR_FALSE); ther This looks like an attempted fix of bug 11683. Eric, can you verify this?
One possible fix: Index: mozilla/webshell/public/nsIWebShell.h =================================================================== RCS file: /cvsroot/mozilla/webshell/public/nsIWebShell.h,v retrieving revision 1.72 diff -r1.72 nsIWebShell.h 420c420 < NS_IMETHOD SetScrolling(PRInt32 aScrolling, PRBool aSetCurrentAndInitial = PR_TRUE) = 0; --- > NS_IMETHOD SetScrolling(PRInt32 aScrolling, PRBool aSetCurrentAndInitial = PR_FALSE) = 0 Index: mozilla/webshell/src/nsWebShell.cpp =================================================================== RCS file: /cvsroot/mozilla/webshell/src/nsWebShell.cpp,v retrieving revision 1.261 diff -r1.261 nsWebShell.cpp 220c220 < NS_IMETHOD SetScrolling(PRInt32 aScrolling, PRBool aSetCurrentAndInitial = PR_TRUE); --- > NS_IMETHOD SetScrolling(PRInt32 aScrolling, PRBool aSetCurrentAndInitial = PR_FALSE); 1793d1792 < mScrolling[1] = aScrolling; 1794a1794 > mScrolling[1] = aScrolling; 1795a1796,1798 > } else { > // Ignore aScrolling and reset scrolling; > mScrolling[1] = mScrolling[0];
*** Bug 15208 has been marked as a duplicate of this bug. ***
*** Bug 12910 has been marked as a duplicate of this bug. ***
*** Bug 15652 has been marked as a duplicate of this bug. ***
*** Bug 15957 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
yes this was to fix bug 11683. When you go to a frameset it disables the scrollbars bug never reenables them. Do you have a better fix?
The problem is that the meaning of the SetScrolling method in the webshell is overloaded. Framesets have been using this method to store their "scrolling" attribute, and now XUL is using it to turn off the scrollbars whenever a XUL document is being displayed. The problem, of course, is that when XUL turns off scrollbars, anyone who checks thinks that this frame/frameset had it's scrolling attribute set to "no", even when a new page is loaded. The fix you used to fix the XUL-stealing-scrollbars bug was to set the value to "no value was specified" every time a new page is loaded. The problem with that is that now whenever anyone checks, they can't see the value for scrolling that the frameset specified. The diff I included above fixes this bug, bit it is a hack, as it just pushes the scrolling state in a 2 level deep stack. I don't have a better solution to this yet, can you think of one?
*** Bug 16705 has been marked as a duplicate of this bug. ***
*** Bug 16384 has been marked as a duplicate of this bug. ***
*** Bug 17307 has been marked as a duplicate of this bug. ***
*** Bug 6724 has been marked as a duplicate of this bug. ***
*** Bug 18807 has been marked as a duplicate of this bug. ***
Summary: scrollbar still visible → FRAME and IFRAME attribute scrolling=no is ignored
Changing Summary to: FRAME and IFRAME attribute scrolling=no is ignored to make it easier to find...
*** Bug 19947 has been marked as a duplicate of this bug. ***
*** Bug 20123 has been marked as a duplicate of this bug. ***
Priority: P3 → P4
Target Milestone: M13
setting p4 for m13. EVaughan, would you please look at Pollman's proposed fix and comment?
I was just about to report this bug. Thats a lot of dupes... I did a search for "frame scroll" and nothing came up. It only came up when I did a search for "frame". Maybe thats why there are so many dupes. No Dupe for you!
*** Bug 21960 has been marked as a duplicate of this bug. ***
*** Bug 21960 has been marked as a duplicate of this bug. ***
Whiteboard: [TESTCASE] scrolling="no" shouldn't be ignored
*** Bug 17652 has been marked as a duplicate of this bug. ***
*** Bug 22470 has been marked as a duplicate of this bug. ***
Blocks: 16346
Target Milestone: M13 → M14
*** Bug 22547 has been marked as a duplicate of this bug. ***
*** Bug 23027 has been marked as a duplicate of this bug. ***
*** Bug 23038 has been marked as a duplicate of this bug. ***
*** Bug 23250 has been marked as a duplicate of this bug. ***
*** Bug 23603 has been marked as a duplicate of this bug. ***
Summary: FRAME and IFRAME attribute scrolling=no is ignored → [DOGFOOD] FRAME and IFRAME attribute scrolling=no is ignored
Adding "DOGFOOD" tag to get a PDT review. This bug has a high dup count and makes many sites look wrong and somewhat unusable.
Can you give us a list of the sites that have this problem? Any of the top 100? -pdt
*** Bug 23825 has been marked as a duplicate of this bug. ***
Whiteboard: [TESTCASE] scrolling="no" shouldn't be ignored → [PDT+][TESTCASE] scrolling="no" shouldn't be ignored
Putting on PDT+ radar.
*** Bug 23949 has been marked as a duplicate of this bug. ***
*** Bug 24348 has been marked as a duplicate of this bug. ***
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase
*** Bug 24805 has been marked as a duplicate of this bug. ***
*** Bug 25675 has been marked as a duplicate of this bug. ***
*** Bug 25598 has been marked as a duplicate of this bug. ***
*** Bug 25890 has been marked as a duplicate of this bug. ***
*** Bug 25954 has been marked as a duplicate of this bug. ***
*** Bug 25971 has been marked as a duplicate of this bug. ***
*** Bug 26071 has been marked as a duplicate of this bug. ***
*** Bug 26157 has been marked as a duplicate of this bug. ***
*** Bug 26535 has been marked as a duplicate of this bug. ***
Putting dogfood in the keyword field.
Keywords: dogfood
*** Bug 26676 has been marked as a duplicate of this bug. ***
Summary: [DOGFOOD] FRAME and IFRAME attribute scrolling=no is ignored → FRAME and IFRAME attribute scrolling=no is ignored
Whiteboard: [PDT+][TESTCASE] scrolling="no" shouldn't be ignored → [PDT+]scrolling="no" shouldn't be ignored
bumping priority to p2
Priority: P4 → P2
Blocks: 26981
Ok this turns out to be a bug in nsHTMLContentSink. If you remove the line: 3012 which shouldn't be there anyway (This was an old hack) xul works without it now. // initially show the scrollbars. We need to do this because another // document like a XUL document, could have have hidden the scrollbars. -EDV //mWebShell->SetScrolling(-1, PR_FALSE); You will find that the bug still happens if you go to page with frames and they and they are set to hidden they will still show up. This is why: 1) When the "Frame" tag that has "scrolling=no" is parsed nsFrameFrame builds a webshell and sets scrolling on it to 1 meaning "no scrollbars" 2) StartLayout is called on the nsHTMLContentSink. 3) At line 3018 it checks to see if mBody is set. Which it is because this is a frame not a frame set. 4) It then gets the nsIMarkupDocumentViewer from the webshell. And asks it if it is a Frame by calling IsFrameDoc. It then returns FALSE which is wrong! It is a frame document. 5) It then proceeds to set scrolling to -1 meaing make scrollbars even though nsFrameFrame has explicitly set it not to have scrollbars. Looks like someone is not telling the nsIMarkupDocumentViewer that is a Frame document. So I figure that when we discover its a frame document we should be calling SetIsFrameDoc(PR_TRUE) nsIMarkupDocumentViewer. Unfortunately the frameset webshell's nsIMarkupDocumentViewer is nsnull. Refering to layout to figure out who might be able to fix this.
Assignee: evaughan → troy
Status: ASSIGNED → NEW
Component: HTMLFrames → Layout
Eric, frameset related so assigning to you
Assignee: troy → pollmann
Blocks: 25824
I see the problem - this is new since the webshell redesign and was masked by the SetScrolling(-1, PR_FALSE) that was just removed. In nsHTMLFrameInnerFrame, where we want call SetIsFrame() we never actually do because the content viewer has not been embeded in the webshell yet. I'll take a look at this after I get finish the other PDT+ bugs I started already. As I just got this today, it will probably take me a day or three to get to it, so I'm guestimating at 16-Feb-2000?
Status: NEW → ASSIGNED
Component: Layout → HTMLFrames
OS: Windows NT → All
Hardware: PC → All
Whiteboard: [PDT+]scrolling="no" shouldn't be ignored → [PDT+] 16-Feb-2000 scrolling="no" shouldn't be ignored
*** Bug 28075 has been marked as a duplicate of this bug. ***
Whiteboard: [PDT+] 16-Feb-2000 scrolling="no" shouldn't be ignored → [PDT+] Fix in hand, need r/a (scrolling="no" shouldn't be ignored)
Reviewed by Vidur and Harish.
Whiteboard: [PDT+] Fix in hand, need r/a (scrolling="no" shouldn't be ignored) → [PDT+] Fix in hand, need approval (scrolling="no" shouldn't be ignored)
Fixed. My fix initially was to re-break bug 11683. It turns out bug 11683 was really simple (nsXULDocument was setting current and initial scroll state when it only should have set current scroll state), so I fixed that too. To verify, go to any of these pages. Shouldn't show scrollbars where they say "scrolling='no'" Also you should see scrollbars after viewing a XUL file (see bug 11683 for a test case.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Whiteboard: [PDT+] Fix in hand, need approval (scrolling="no" shouldn't be ignored) → [PDT+] (scrolling="no" shouldn't be ignored)
Fixed in the Feb 21 build.
Status: RESOLVED → VERIFIED
*** Bug 29769 has been marked as a duplicate of this bug. ***
Product: Core → Core Graveyard
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: