Bug 10517 – REGRESSION (r12065-r12082): Navigation graphic wraps to the next line on duart.com

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

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.

Created attachment 10258 [details]
Reduction

WinIE, Firefox and Opera lay out the two images on the same line.

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.

[Stephanie Lewis]

radar 4823092

*** Bug 12456 has been marked as a duplicate of this bug. ***

Bug 12456 lists another affected site <http://www.microsoft.com/windows/products/windowsvista/default.mspx> and contains another reduction (attachment 12772 [details]).

It looks like I broke it in <http://trac.webkit.org/projects/webkit/changeset/12073> :(

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).

Created attachment 12778 [details]
More weird Firefox behavior

Firefox applies the quirk only to the first line, or to the first "non-breakable" run.

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.

[Beth Dakin]

I talked to Hyatt about this, and I'm working on a patch.

[Beth Dakin]

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.

[Dave Hyatt]

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.

[Beth Dakin]

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.

[Beth Dakin]

Created attachment 12863 [details]
New patch

Here is a new patch after working through this a bunch with Dave.

[Dave Hyatt]

Comment on attachment 12863 [details]
New patch

r=me

[Beth Dakin]

Fixed in r19342.

*** Bug 5066 has been marked as a duplicate of this bug. ***

*** Bug 9832 has been marked as a duplicate of this bug. ***