Bug 10517 - REGRESSION (r12065-r12082): Navigation graphic wraps to the next line on duart.com
Summary: REGRESSION (r12065-r12082): Navigation graphic wraps to the next line on duar...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Beth Dakin
URL: http://www.duart.com/site_main.html
Keywords: HasReduction, InRadar, Regression
: 5066 9832 12456 (view as bug list)
Depends on:
Reported: 2006-08-22 14:13 PDT by mitz
Modified: 2007-03-31 13:10 PDT (History)
4 users (show)

See Also:

Reduction (224 bytes, text/html)
2006-08-27 10:02 PDT, mitz
no flags Details
Reduction (regressed case) (214 bytes, text/html)
2006-08-27 10:05 PDT, mitz
no flags Details
More weird Firefox behavior (799 bytes, text/html)
2007-01-29 23:38 PST, mitz
no flags Details
WinIE rendering of attachment 12778 (7.99 KB, image/png)
2007-01-29 23:48 PST, mitz
no flags Details
Patch to implement quirk (25.46 KB, patch)
2007-01-31 22:11 PST, Beth Dakin
hyatt: review-
Details | Formatted Diff | Diff
New patch (47.30 KB, patch)
2007-02-01 16:38 PST, Beth Dakin
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2006-08-22 14:13:23 PDT
The "titles+fx" piece of the navigation graphic at the top left of the page doesn't fit on the line with the other pieces.

This is a regression from shipping Safari. Firefox also renders correctly.
Comment 1 mitz 2006-08-27 10:02:20 PDT
Created attachment 10258 [details]

WinIE, Firefox and Opera lay out the two images on the same line.
Comment 2 mitz 2006-08-27 10:05:59 PDT
Created attachment 10259 [details]
Reduction (regressed case)

In this case, the two images fit on the same line in shipping Safari, but not in ToT.
Comment 3 Stephanie Lewis 2006-11-06 20:52:02 PST
radar 4823092
Comment 4 mitz 2007-01-29 22:39:50 PST
*** Bug 12456 has been marked as a duplicate of this bug. ***
Comment 5 mitz 2007-01-29 22:41:54 PST
Bug 12456 lists another affected site <http://www.microsoft.com/windows/products/windowsvista/default.mspx> and contains another reduction (attachment 12772 [details]).
Comment 6 mitz 2007-01-29 22:47:44 PST
It looks like I broke it in <http://trac.webkit.org/projects/webkit/changeset/12073> :(
Comment 7 mitz 2007-01-29 23:09:57 PST
I think that the first reduction is the real issue here: in quirks mode, Firefox and Opera don't allow breaks between images in table cells and they grow the cell. WebKit does not have that quirk. The only reason the regressions happened is that in those cases the overflow was 1px and prior to the bugfix in r12073, the calculation was off by 1 (so the overflow went unnoticed, the line break didn't happen but the cell didn't grow either).
Comment 8 mitz 2007-01-29 23:38:28 PST
Created attachment 12778 [details]
More weird Firefox behavior

Firefox applies the quirk only to the first line, or to the first "non-breakable" run.
Comment 9 mitz 2007-01-29 23:48:22 PST
Created attachment 12779 [details]
WinIE rendering of attachment 12778 [details]

WinIE renders attachment 12778 [details] the same in strict mode and in quirks mode. It seems to follow a simple rule that does not allow breaks between an image and text or another image.
Comment 10 Beth Dakin 2007-01-31 16:23:31 PST
I talked to Hyatt about this, and I'm working on a patch.
Comment 11 Beth Dakin 2007-01-31 22:11:59 PST
Created attachment 12848 [details]
Patch to implement quirk

Here is a patch. It's kind of insane in its specificity, but I don't know that there is any other way to implement this quirk and get it right. The changes in the Layout test directory show one new test, and three tests with new results. Two of the tests with new results were failing mozilla tests that now pass. The third test with new results was considered "passing" before. The new results look almost identical to the old ones (which is why we considered it to be passing), but when you look at the diff, the new results are clearly more correct.
Comment 12 Dave Hyatt 2007-01-31 22:23:34 PST
Comment on attachment 12848 [details]
Patch to implement quirk

You can use style->htmlHacks() to ask if you're in quirks mode in a more compact way.

The sibling checks are flawed, since content can be nested.  That's why there's an iterator that was drilling into and out of nested inline flows like spans and such to find all the text/image leaves.  I'm not sure why the sibling checks are necessary.

I also don't understand the table cell width check, since that should be factored into min/max width later on.

Maybe we can look at this in person tomorrow to see what the failures were with a less specific version of the quirk.
Comment 13 Beth Dakin 2007-01-31 22:29:39 PST
The table cell width check is probably just wrong; I added that in my first iteration of the patch, not for any specific bug that I saw. The siblings stuff we will have to sit down together and look at. I would love for there to be a better way to fix those cases.
Comment 14 Beth Dakin 2007-02-01 16:38:59 PST
Created attachment 12863 [details]
New patch

Here is a new patch after working through this a bunch with Dave.
Comment 15 Dave Hyatt 2007-02-01 16:43:44 PST
Comment on attachment 12863 [details]
New patch

Comment 16 Beth Dakin 2007-02-01 17:05:51 PST
Fixed in r19342.
Comment 17 mitz 2007-02-02 07:08:51 PST
*** Bug 5066 has been marked as a duplicate of this bug. ***
Comment 18 mitz 2007-03-31 13:10:33 PDT
*** Bug 9832 has been marked as a duplicate of this bug. ***