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)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
VERIFIED
FIXED
M14
People
(Reporter: gemma, Assigned: pollmann)
References
()
Details
(Keywords: testcase, Whiteboard: [PDT+] (scrolling="no" shouldn't be ignored))
Attachments
(1 file)
(deleted),
text/html
|
Details |
for frames with scrolling="no" still have the scroll bar visible.
Updated•25 years ago
|
Assignee: karnaze → pollmann
Comment 1•25 years ago
|
||
Eric, this must be a regression.
Assignee | ||
Updated•25 years ago
|
Assignee: pollmann → evaughan
Assignee | ||
Comment 2•25 years ago
|
||
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?
Assignee | ||
Comment 3•25 years ago
|
||
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];
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 8•25 years ago
|
||
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?
Assignee | ||
Comment 9•25 years ago
|
||
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?
Comment 10•25 years ago
|
||
*** Bug 16705 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 11•25 years ago
|
||
*** Bug 16384 has been marked as a duplicate of this bug. ***
Comment 12•25 years ago
|
||
*** Bug 17307 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 13•25 years ago
|
||
*** Bug 6724 has been marked as a duplicate of this bug. ***
Comment 14•25 years ago
|
||
*** Bug 18807 has been marked as a duplicate of this bug. ***
Updated•25 years ago
|
Summary: scrollbar still visible → FRAME and IFRAME attribute scrolling=no is ignored
Comment 15•25 years ago
|
||
Changing Summary to:
FRAME and IFRAME attribute scrolling=no is ignored
to make it easier to find...
Assignee | ||
Comment 16•25 years ago
|
||
*** Bug 19947 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 17•25 years ago
|
||
*** Bug 20123 has been marked as a duplicate of this bug. ***
Updated•25 years ago
|
Priority: P3 → P4
Target Milestone: M13
Comment 18•25 years ago
|
||
setting p4 for m13. EVaughan, would you please look at Pollman's proposed fix
and comment?
Comment 19•25 years ago
|
||
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!
Assignee | ||
Comment 20•25 years ago
|
||
*** Bug 21960 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 21•25 years ago
|
||
*** Bug 21960 has been marked as a duplicate of this bug. ***
Comment 22•25 years ago
|
||
Comment 23•25 years ago
|
||
*** Bug 17652 has been marked as a duplicate of this bug. ***
Comment 24•25 years ago
|
||
*** Bug 22470 has been marked as a duplicate of this bug. ***
Updated•25 years ago
|
Target Milestone: M13 → M14
Assignee | ||
Comment 25•25 years ago
|
||
*** Bug 22547 has been marked as a duplicate of this bug. ***
Comment 26•25 years ago
|
||
*** Bug 23027 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 27•25 years ago
|
||
*** Bug 23038 has been marked as a duplicate of this bug. ***
Comment 28•25 years ago
|
||
*** Bug 23250 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 29•25 years ago
|
||
*** 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
Comment 30•25 years ago
|
||
Adding "DOGFOOD" tag to get a PDT review. This bug has a high dup count
and makes many sites look wrong and somewhat unusable.
Comment 31•25 years ago
|
||
Can you give us a list of the sites that have this problem? Any of the top 100?
-pdt
Comment 32•25 years ago
|
||
URLs from the "top 100" which exhibit this problem:
Number 16: http://www.mysap.com/
Number 93: http://www.as.org/
Number 121: http://www.thestreet.com/
URLs collected from the duplicate reports also showing this bug:
http://www.linuxfreak.ch/linuxfreak.htm
http://economy.tos.net/
http://www.projo.com/
http://www.fragtastic.com/
http://members.tripod.com/~adeadend/index2.htm
http://gamma.nic.fi/~tapio1/index_frames.html
http://www.bit-forge.com/woa2/
http://www.sbs.co.kr/
http://www.stvlive.com/
http://y2k.2000.ru/
http://www.mis3.udel.edu:90/Events/CampusEvents.html
http://www.openvrml.org/
http://www.omega.ch/n2_bond.html
Assignee | ||
Comment 33•25 years ago
|
||
*** 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
Comment 34•25 years ago
|
||
Putting on PDT+ radar.
Comment 35•25 years ago
|
||
*** Bug 23949 has been marked as a duplicate of this bug. ***
Comment 36•25 years ago
|
||
*** Bug 24348 has been marked as a duplicate of this bug. ***
Comment 37•25 years ago
|
||
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase
Comment 38•25 years ago
|
||
*** Bug 24805 has been marked as a duplicate of this bug. ***
Comment 39•25 years ago
|
||
*** Bug 25675 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 40•25 years ago
|
||
*** Bug 25598 has been marked as a duplicate of this bug. ***
Comment 41•25 years ago
|
||
*** Bug 25890 has been marked as a duplicate of this bug. ***
Comment 42•25 years ago
|
||
*** Bug 25954 has been marked as a duplicate of this bug. ***
Comment 43•25 years ago
|
||
*** Bug 25971 has been marked as a duplicate of this bug. ***
Comment 44•25 years ago
|
||
*** Bug 26071 has been marked as a duplicate of this bug. ***
Comment 45•25 years ago
|
||
*** Bug 26157 has been marked as a duplicate of this bug. ***
Comment 46•25 years ago
|
||
*** Bug 26535 has been marked as a duplicate of this bug. ***
Comment 48•25 years ago
|
||
*** Bug 26676 has been marked as a duplicate of this bug. ***
Updated•25 years ago
|
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
Comment 50•25 years ago
|
||
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
Assignee | ||
Comment 52•25 years ago
|
||
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
Comment 53•25 years ago
|
||
*** Bug 28075 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•25 years ago
|
Whiteboard: [PDT+] 16-Feb-2000 scrolling="no" shouldn't be ignored → [PDT+] Fix in hand, need r/a (scrolling="no" shouldn't be ignored)
Assignee | ||
Comment 54•25 years ago
|
||
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)
Assignee | ||
Comment 55•25 years ago
|
||
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
Assignee | ||
Updated•25 years ago
|
Whiteboard: [PDT+] Fix in hand, need approval (scrolling="no" shouldn't be ignored) → [PDT+] (scrolling="no" shouldn't be ignored)
Assignee | ||
Comment 57•25 years ago
|
||
*** Bug 29769 has been marked as a duplicate of this bug. ***
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
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.
Description
•