Closed Bug 227567 Opened 21 years ago Closed 21 years ago

[FIX] displays incorrectly

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: cotton_bill, Assigned: MatsPalmgren_bugz)

References

(
URL
)

Details

(Keywords: compat)

Attachments

(1 file, 2 obsolete files)

Patch rev. 3
(deleted), patch
bzbarsky
: review+
jst
: superreview+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031030 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031030 The "footer" div is displayed at the top of the page and obscures the top part of the page. The "side menu" div is displayed with bottom half of text missing. Works OK in IE Reproducible: Always Steps to Reproduce: 1.request the page 2. 3. Actual Results: duff display Expected Results: good display!
> The "footer" div is displayed at the top of the page It's absolutely positioned, with its top 110px from the top of the page by the following code: document.getElementById('footer').style.top=document.getElementById('sidemenu').clientHeight+110 In mozilla the "clientHeight" is 0, since the element is not scrollable... I assume IE makes clientHeight equal offsetHeight or something when the element is not scrollable? (the sidemenu div is likely affected by the same problem, by the way).
Assignee: other → general
Component: Layout → DOM: Mozilla Extensions
OS: Windows XP → All
Hardware: PC → All
The overlap in the sidemenu is the correct rendering. It is the result of a specified 5px vertical padding on the inline elements (links) in the menu. http://www.w3.org/TR/CSS21/visudet.html#inline-non-replaced "The vertical padding, border and margin of an inline, non-replaced box start at the top and bottom of the content area, not the 'line-height'. But only the 'line-height' is used to calculate the height of the line box."
Attached patch Patch rev. 1 (obsolete) (deleted) — — Splinter Review
This was originally fixed for <body> in bug 180552. I don't see any reason why it shouldn't be fixed for other elements too.
Mats, does that give us compat with IE as far as clientHeight goes? For example, what does IE report as the clientHeight for non-positioned non-scrolling elements? If this is what IE does, the patch looks great!
Attachment #136913 - Attachment is obsolete: true
It seems IE isn't as general as that, it returns zero for anything that isn't a float or abs.pos. or replaced or table element or <body> or <frameset>. I have a patch that does that, I just need to test IE a bit more exactly first...
Assignee: general → mats.palmgren
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: compat
Aiie. One of these days we'll discover that one of IE's little DOM extensions is implemented in a sane and consistent fashion and the world will end....
Is "0" as a return value ever used by anyone? If not, I say "embrace and extend" and just return the right height all the time. :-)
It's even worse than I described in comment 5, for example <div>X</div> returns zero but <div style="height:100px">X</div> returns 100. So it seems it is dependent on non-auto height in some cases (normal flow or relative pos.) but not in other cases (floats and abs.pos). Speculation: I think clientHeight/Width in IE returns the dimension of some sort of internal "view". This is impossible to emulate exactly of course.
Attached patch Patch rev. 2 (obsolete) (deleted) — — Splinter Review
This is a compromise that I think will cover most cases in a compatible way...
Attached patch Patch rev. 3 (deleted) — — Splinter Review
Oops, didn't mean to remove the NULL check on 'frame'.
Attachment #136940 - Attachment is obsolete: true
Summary: displays incorrectly → [FIX] displays incorrectly
Comment on attachment 136941 [details] [diff] [review] Patch rev. 3 It returns the right height almost "all the time" as Ian said - the exception being non-replaced inline elements.
Attachment #136941 - Flags: review?(bz-vacation)
> Is "0" as a return value ever used by anyone? Ian, this is the web. Yes, there are pages that rely on that behavior. Probably thousands of them. :(
Comment on attachment 136941 [details] [diff] [review] Patch rev. 3 I agree that emulating this in all its quirky glory is probably impossible... Note that the documentation at http://msdn.microsoft.com/workshop/author/dhtml/reference/properties/clientheig ht.asp is totally useless. This seems reasonable to me. r=bzbarsky. jst, would you sr?
Attachment #136941 - Flags: superreview?(jst)
Attachment #136941 - Flags: review?(bz-vacation)
Attachment #136941 - Flags: review+
I find it very hard to believe that anyone is actually depending on that. Can you show me an example? Do we actually care about those cases? Making ourselves appear more reliable and less quirky goes a long way towards making authors prefer Mozilla to IE.
Blocks: 156388
Ian, I think that's actually what Neil's patch does... returning a clientHeight for non-replaced inlines would be very odd, imo.
I added the exception for non-replaced inlines for two reasons: 1. this is what IE does (and this bug is about being IE compat) 2. CSS height property does not apply to those elements (so there is a kind vague symmetry there ;-)
Yeah, I guess that makes sense. FWIW, there _is_ a height defined for inline elements, it's the content height. I'm not 100% sure what offsetHeight gives back, but the height from margin edge to margin edge of an inline element is well defined. (Vertically at least; horizontally it should probably be the sum of all the boxes, although who knows.) But yes, I see the logic of excluding them. I retract my objections. :-)
Attachment #136941 - Flags: superreview?(jst) → superreview+
Mats, please poke me for checkin if this gets approved or the tree reopens, ok?
Tree is open, please check it in.
checked in. Checking in content/html/content/src/nsGenericHTMLElement.cpp; /cvsroot/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,v <-- nsGenericHTMLElement.cpp new revision: 1.458; previous revision: 1.457 done
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
In All-in-One gestures extension, I test clientHeight and clientWidth to check whether a node is scrollable or not. Should I assume you broke that with the patch? Is there a way to workaround?
You can probably test whether client* is equal to offset*; if they are not equal, you can scroll, no?
Not so easy when client* doesn't include the borders while offset* (or scroll* which I use) does.
So you can just compare offset* to scroll* then, no? See whether they are the same? In any case, compat with IE on properties introduced by IE is really what we're aiming for...
Boris, AFAIK, offset* and scroll* are the same. However, I managed to get the border values via getComputedStyle and add them to client* before comparing to offset*. Seems to work though quite hacky.
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: