Summary: | exeter.edu renders improperly | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Will Stoutin <willstoutin> | ||||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | NEW --- | ||||||||||||||||
Severity: | Normal | CC: | bfulgham, carlamichele, emacemac7, sam, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | HasReduction, InRadar | ||||||||||||||
Version: | 420+ | ||||||||||||||||
Hardware: | Mac | ||||||||||||||||
OS: | OS X 10.4 | ||||||||||||||||
URL: | http://www.exeter.edu | ||||||||||||||||
Attachments: |
|
Description
Will Stoutin
2006-07-06 20:48:11 PDT
Created attachment 9246 [details]
reduced test case
Adding a reduced test case to illustrate the difference between ToT and Firefox.
I see this too. Reporter, you should file one bug report for each issue you see, unless you are certain they all have the same root cause. Since Sam Weinig already added a test case for the wrong logo placement, I think it would be good to only track that bug here, and that you should file new bugs for the other issues. Created attachment 9291 [details]
Slightly more reduced test case
I reduced Sam's reduction slightly, without removing the issue.
Created attachment 9373 [details]
Patch, including layout test and change log
Is this a regression from production Safari? If so, it needs to be P1 and have a NeedsRadar keyword added. This issue is present in the current production safari (for 10.4.7, 419.3), so this is not a regression. Leaving it at P2. Comment on attachment 9373 [details]
Patch, including layout test and change log
This patch is actually wrong. You should pass nonAnonymousContainingBlock()->contentHeight() to calcValue() instead of containingBlockHeight().
Created attachment 11504 [details]
updated patch
This updates mitz's patch based on his comments.
Comment on attachment 11504 [details]
updated patch
r- due to some ChangeLog issues I discussed with Sam on IRC.
Created attachment 11511 [details]
patch with new ChangeLog
Updated patch. Someone should really test this before landing as my computer is rather on the fritz and the results have been rather inconsistent.
(In reply to comment #10) > Someone should really test this before landing as my computer > is rather on the fritz and the results have been rather inconsistent. > I applied the patch to my tree and verified that there are no layout test regressions (and that the new test passes). I'm not so sure about this. Why should anonymous blocks be skipped? (In reply to comment #12) > I'm not so sure about this. Why should anonymous blocks be skipped? > We talked about it on IRC. The spec doesn't say anything about it, but Firefox and Opera do it even in strict mode. However, the patch is incomplete since it addresses percentage 'top' and 'bottom' but not percentage 'height' (the latter skips anonymous blocks in quirks mode, but only because it skips all block with height: auto in that mode); starting from the nonAnonymousContainingBlock in calcPercentageHeight should solve that. The layout test should cover percentage height. Comment on attachment 11511 [details]
patch with new ChangeLog
r- due to the above
Created attachment 14619 [details]
screenshot of rendering error
showing the error reported as of build 420+
I'm not so certain this is a WebKit bug, per se, since I just validated the webpage in question, and it failed with 140 XHTML errors (http://validator.w3.org/check?uri=http%3A//www.exeter.edu/). There are also several CSS errors (http://jigsaw.w3.org/css-validator/validator?uri=http%3A//www.exeter.edu/), including the repeated substitution of "font-name" for "font-family". (In reply to comment #13) > The spec doesn't say anything about it Actually, I think the spec is pretty clear: "if the element's position is 'relative' or 'static', the containing block is formed by the content edge of the nearest *block-level*, table cell or inline-block ancestor box" (<http://www.w3.org/TR/2006/WD-CSS21-20061106/visudet.html#containing-block-details>). Generating an anonymous block box doesn't make you a block-level element. I think containingBlock() should just be changed to not return anonymous blocks in this case. The problem is that some callers expect the current behavior, so I think containingBlock() could have an allowAnonymousBlocks parameter that defaults to false, and callers that need to see anonymous boxes could be changed to pass true for that parameter. A slightly less risky approach for now would be to make the default 'true' and just patch the few callers that we know that need to skip anonymous blocks (currently relativePositionOffsetY and calcPercentageHeight). This continues to be an issue in Safari 16. |