Bug 10517

Summary: REGRESSION (r12065-r12082): Navigation graphic wraps to the next line on duart.com
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, gabriel, nordholm, webkit-bugs
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.duart.com/site_main.html
Attachments:
Description Flags
Reduction
none
Reduction (regressed case)
none
More weird Firefox behavior
none
WinIE rendering of attachment 12778
none
Patch to implement quirk
hyatt: review-
New patch hyatt: review+

mitz
Reported 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.
Attachments
Reduction (224 bytes, text/html)
2006-08-27 10:02 PDT, mitz
no flags
Reduction (regressed case) (214 bytes, text/html)
2006-08-27 10:05 PDT, mitz
no flags
More weird Firefox behavior (799 bytes, text/html)
2007-01-29 23:38 PST, mitz
no flags
WinIE rendering of attachment 12778 (7.99 KB, image/png)
2007-01-29 23:48 PST, mitz
no flags
Patch to implement quirk (25.46 KB, patch)
2007-01-31 22:11 PST, Beth Dakin
hyatt: review-
New patch (47.30 KB, patch)
2007-02-01 16:38 PST, Beth Dakin
hyatt: review+
mitz
Comment 1 2006-08-27 10:02:20 PDT
Created attachment 10258 [details] Reduction WinIE, Firefox and Opera lay out the two images on the same line.
mitz
Comment 2 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.
Stephanie Lewis
Comment 3 2006-11-06 20:52:02 PST
radar 4823092
mitz
Comment 4 2007-01-29 22:39:50 PST
*** Bug 12456 has been marked as a duplicate of this bug. ***
mitz
Comment 5 2007-01-29 22:41:54 PST
mitz
Comment 6 2007-01-29 22:47:44 PST
mitz
Comment 7 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).
mitz
Comment 8 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.
mitz
Comment 9 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.
Beth Dakin
Comment 10 2007-01-31 16:23:31 PST
I talked to Hyatt about this, and I'm working on a patch.
Beth Dakin
Comment 11 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.
Dave Hyatt
Comment 12 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.
Beth Dakin
Comment 13 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.
Beth Dakin
Comment 14 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.
Dave Hyatt
Comment 15 2007-02-01 16:43:44 PST
Comment on attachment 12863 [details] New patch r=me
Beth Dakin
Comment 16 2007-02-01 17:05:51 PST
Fixed in r19342.
mitz
Comment 17 2007-02-02 07:08:51 PST
*** Bug 5066 has been marked as a duplicate of this bug. ***
mitz
Comment 18 2007-03-31 13:10:33 PDT
*** Bug 9832 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.