Closed Bug 260652 Opened 21 years ago Closed 21 years ago

frameid.document..body.clientHeight is wrong in frames with scrolling="no"

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: fotemac, Unassigned)

References

(
URL
)

Details

User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040616 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040616 The frameid.document.body.clientHeight value returned via javascript is always wrong in frame sets for any frames which have scrolling="no" The value returned is correct if scrolling is "yes" or "auto" The URL provided is for a simple frame set with shows this bug. The bug occurs in every real world frame set that I have tried, for all versions of Mozilla through recent nightly builds. Reproducible: Always Steps to Reproduce: 1.Set scrolling="no" for any frame in a frame set. 2.The frameid.document.body.clientHeight value for that frame will become garbage. 3. Actual Results: value became wrong Expected Results: value should be correct as when scrolling is set to "yes" or "auto"
Worksforme in a current Linux trunk build -- all frames show the same clientheight for the body. Foteos Macrides, could you please test a trunk build? It if still fails for you, please clearly describe what output you see?
(In reply to comment #1) > Worksforme in a current Linux trunk build -- all frames show the same > clientheight for the body. Foteos Macrides, could you please test a trunk > build? It if still fails for you, please clearly describe what output you see? I just downloaded and tried Mozilla nightly build rv 1.8a4 Gecko/20040922 on Win98. It has the same problem as all earlier verions that I tried. In each of the three frames at the above URL, a "show" link calls a javascript function which uses alert() to show the clientWidth / clientHeight and the innerWidth / innerHeight values for each of the three frames. In the frames with scrolling set to "yes" or "auto" (and document content set up to yield both horizontal and vertical scrollbars) the sets of values differ by 16, corresponding to the "narrow dimension" of the scrollbars. In the frame with scrolling="no" the sets of values should be the same (not differ by 16) because there are no scrollbars. In that case, clientWidth does equal innerWidth, and innerHeight is correct, but clientHeight is wrong (and thus, not equal to innerHeight, as it should be). The actual values of course will depend on your overall window size and how much of it is used by the chrome. If you used a window size on your Linux system such that the middle and right frame have scrollbars, and the left frame of course doesn't because it has scrolling="no", then you should not show the same clientHeight for all three frames, as you claim. The clientHeight for the left frame should be more than that of the middle and right frame by the height of a horizontal scollbar (perhaps 16 on your Linux system, as on my Win98 system). But the wrong value for clientHeight when scrolling="no" on my Win98 system is much more than a difference 16. It is always very far away from the correct value (e.g., 1022 when it should be 440). As I stressed when filing the bug report, on my Win98 system clientHeight is wrong in any frame which has scrolling="no" for every frame set that I've tried, not just the example I provided via the above URL. In my real world frame sets, I've added the workaround of testing (frameid.scrollbars&&!frameid.scrollbars.visible) for the Geckos, i.e., whether scrolling="no", and using innerHeight in that case because it is correct and what clientHeight should be in the absence of a horizontal scrollbar.
Oh, I see what's happening here... first of all, when you ask for the clientHeight of the body, in quirks mode, we look at the "document" (well, viewport, or frame for subframes) instead. We get its scroll info, etc. In standards mode, we would actually return the height of the body, which is taller than the frame in your case. Now in this case, the frame has no scroll info (since it's not scrollable), so we fall back on the actual size of the <body> (see bug 180552 for the reasons). So the quirk you're relying on doesn't work. Now the fact that we have no scrollframe there is insteresting. Is that intended? scrolling="no" is mapped to overflow:hidden on the frame, which is used to set the frame's docshell's scrollbar prefs to Scrollbar_Never in both directions... is that killing off our scrollframe somehow?
It shouldn't be. I thought that we'd fixed things in nsCSSFrameConstructor::ConstructRootFrame to always build a scrollframe.
Well, nsGenericHTMLElement::GetScrollInfo is returning a null scrollable view in this case... (see impl of nsGenericHTMLElement::GetClientHeight). Given that the page is in quirks and that we should hence be walking the parents of the body in GetScrollInfo(), something is fishy.
(In reply to comment #3) > Oh, I see what's happening here... <snip> in quirks mode, <snip> Here's what might be relevant information about quirks versus other modes (with the Mozilla 20040922 nightly build on my Win98 system). You can change the frameset document (i.e., that contains the frameset and frame elements) to the Gecko's "almost standards mode" by adding a system identifier (URL) for frameset.dtd and that doesn't change anything I've reported. However, if you make the documents within the frames almost standard by adding system identifiers for loose.dtd, then with my "three columns of frames" test frame set the documents with scrolling set to "yes" or "auto" will have the same wrong value for clientHeight as does the document with scrolling="no" (clientWidth, innerWidth and innerHeight remain correct). The values for clientHeight may differ from each other with other organizations of frame sets, but will be wrong across the board in almost standards mode. The same is true (clientHeight becomes wrong across the board, while the other three values remain correct) if you make the documents full standards mode (strict.dtd). So the Geckos work best for getting correct values of clientHeight with frame documents in quirks mode, and yield total garbage, across the board, for clientHeight in the other modes.
> and yield total garbage No, it yields the height of the body. After all, that's what body.clientHeight is defined to be (in the MSDN documentation). Note that IE6 in standards mode behaves the same way as Mozilla in standards mode for body.clientHeight -- returns the actual height of the body. In quirks mode both return some totally different number (the height of the viewport, to be precise).
(In reply to comment #7) The msdn documentation for clientHeight is: http://msdn.microsoft.com/library/default.asp? url=/workshop/author/dhtml/reference/properties/clientheight.asp The markup for its example is quirks mode, and the example compares clientHeight with offsetHeight for a div with lenthy content and scolling="auto" such that scrollbars are present. The value returned for clientHeight is the "viewport" for the element (a div), 16 less than the value returned for offsetHeight because the latter includes the scrollbars (homologously to innerHeight). The value is not a much larger one corresponding to the overall length of the element's content. I believe that's what is intended as well when the element is body. I have expanded my quirks mode test frame set: http://www.macridesweb.com/oltest/frameDimsBug.html so that is also shows offsetWidth and offsetHeight. I also have put up a homologous test frame set in almost standards mode (loose.dtd): http://www.macridesweb.com/oltest/frameDimsBugAlmStd.html and another in full standards mode (strict.dtd): http://www.macridesweb.com/oltest/frameDimsBugStd.html Please look carefully at what values different browsers return for those three frame sets before inferring that the browsers are doing the same or different things. As far as Mozilla is concerned, in all three frames of all three sets with different modes, for clientWidth it is returning a value corresponding to the viewport width (16 less than the values for offsetWidth and innerWidth). The overall width of the body based on the markup is much larger than that, and is not being used for clientWidth in any case. In contrast, in some cases the value of clientHeight does get returned as the overall height of the body, rather than the viewport height. So either the clientWidth handling or the clientHeight handling has a bug. I believe the bug is in the handling of clientHeight. If you have Opera 7.5+, try that browser with my three test frame sets. It returns the viewport for both clientWidth and clientHeight for all three frames across all three sets with the DOCTYPEs that are treated as three different modes by Mozilla. I'm sure the Mozilla developers can do just as well.
> I believe that's what is intended as well when the element is body. You're confusing the body's scrollbars with the viewport/frame scrollbars. They're not the same thing, except in quirks mode. In particular, it is possible to put separate scrollbars on both <html> and <body> in Mozilla. Try this: put borders on both the div and the body in all your testcases to see the actual sizes of the elements and compare the clientWidth/Height values to the sizes of the border-boxes minus the sizes of the borders and scrollbars. > The overall width of the body based on the markup is much larger than that No, it's not. The body just overflows in the horizontal direction. Again, put a border on the body to see this. In all standards mode cases, the clientWidth/Height will reflect the actual size of the body (ignoring any overflow). > I'm sure the Mozilla developers can do just as well. The point is, that's not what IE6 returns (IE fails to render the right width for the body due to bugs in its CSS implementation, but the heights it returns in standards mode correspond to the actual body height, not the height of the frame the body is in). Since IE is what defines the client* properties, we should be doing something as close as possible to what IE does. Note that I agree that scrolling="no" should not affect the clientHeight of the body in quirks mode. The fact that it does is wrong (because in IE it does not). It's still not clear to me why we don't end up with a scrollframe in that case...
(In reply to comment #9) I'm glad you agree that Mozilla'a handling of clientHeight in quirks mode when scrolling="no" is a bug, and hopefully that can be fixed soon. It would seem to follow that Mozilla's handling of offsetHeight is an equivalent bug, but they may be co-dependent so perhaps specific consideration of offsetHeight can be put off until the clientHeight bug is fixed. I tried adding colored borders and additional scrollbars ("to see better what is going on" as you suggested) via these CSS rules; html {margin:0px; border:solid red; overflow:scroll;} body {margin:0px; border:solid blue; overflow:scroll;} I've put out the additional test frame sets with those rules for quirks mode: http://www.macridesweb.com/oltest/frameDimsBugBO.html almost standards mode (loose.dtd): http://www.macridesweb.com/oltest/frameDimsBugAlmStdBO.html and full standards mode (strict.dtd): http://www.macridesweb.com/oltest/frameDimsBugStdBO.html At this point, I won't go into what they seem to show (beyond that Mozilla 1.8a4, IE 6.0, and Opera 7.5 all behave differently from each other on my Win98 system). I'm simply letting you know that they are available as additional test cases.
(In reply to comment #9) > The point is, that's not what IE6 returns I see that Mozilla 1.8a4 has an emulation of IE's compatMode / documentElement. Looks like it was added at some point between the 1.4 and 1.7 milestones. I modified the above test frame sets accordingly, i.e., they use documentElemnt instead of body when compatMode is CSS1Compat. The scrolling="no" case is still problematic across the board for Mozilla.
Over to layout misc. roc, the problem here is that nsScrollPortFrame (which just inherits from nsScrollBoxFrame) claims to be an nsLayoutAtoms::scrollFrame, but doesn't implement nsIScrollableFrame. If you look at nsCSSFrameConstructor::ConstructRootFrame, we construct an nsScrollPortFrame for the "non-scrollable HTML" case (around line 3976 on current trunk). Now "non-scrollable" happens for HTML in two cases: 1) Frameset document (see line 3853) 2) scrolling="no" frame (see comment at line 3856 and ensuing block of code). I've verified that removing this latter chunk of code fixes this bug. I rather doubt this is still needed "for performance", is it? What about the frameset case? Note that constructing nsScrollPortFrame here makes us assert in PresShell::EndLoad. Try the following minimal testcase for the assert: data:text/html,<frameset> I think the right thing to do is to never create these nsScrollPortFrame beasties for root scrollframes (and create actual scrollframes instead). If framesets need fixing after that, we should fix them.
Assignee: general → nobody
Severity: normal → major
Status: UNCONFIRMED → NEW
Component: DOM → Layout: Misc Code
Ever confirmed: true
OS: Windows 98 → All
QA Contact: ian → core.layout.misc-code
Hardware: PC → All
> I rather doubt this is still needed "for performance", is it? dbaron wrote that comment, better ask him... I agree that constructing real scrollframes seems like the right way to go. Especially because I'm hoping to completely eliminate nsScrollBoxFrame soon...
Note: the frameset issue may cause a workaround to be checked in for bug 262578
Blocks: 262578
Removing that chunk of code was part of my patch for bug 72747, but it caused a Tp regression (and only a little more than a month ago). See bug 72747 comment 36 and revisions 1.984 through 1.987 of nsCSSFrameConstructor.cpp.
For reference, the numbers on btek were: 52316 2004:09:03:23:30:16 819 52317 2004:09:03:23:47:44 817 52318 2004:09:04:00:05:21 817 landed rev 1.984: 52319 2004:09:04:01:05:12 830 52320 2004:09:04:01:22:36 829 52321 2004:09:04:01:39:47 825 landed rev 1.985: 52322 2004:09:04:02:03:03 818 52323 2004:09:04:02:20:30 819 52324 2004:09:04:02:38:11 816 The next two revisions didn't seem to cause noticeable changes.
related to bug 108591 (just guessing) ?
My checkin for bug 288117 should have fixed this.
Depends on: 288117
Fixed by (re-)checkin for bug 288117
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.