Bug 8495

Summary:

REGRESSION: Sidebar on cnn.com is hosed

Product:

WebKit

Reporter:

Dave Hyatt <hyatt>

Component:

Layout and Rendering

Assignee:

Beth Dakin <bdakin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, c.petersen87, henk.kampman

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

URL:

http://www.cnn.com/SHOWBIZ/

Attachments:

Description	Flags
Reduction	none
Patch	hyatt: review-
Still not right	none
I think this works...	hyatt: review+

Dave Hyatt

Reported 2006-04-19 22:49:10 PDT

Go to: http://www.cnn.com/SHOWBIZ/ The categories in the sidebar should have a solid dark blue background. Instead there's some weird lighter blue mixed in. The sidebar looks totally garbled as a result. Shipping Safari looks fine.

Attachments
Reduction (247 bytes, text/html) 2006-04-20 10:10 PDT, Beth Dakin	no flags	Details
Patch (1.40 KB, patch) 2006-04-20 22:06 PDT, Beth Dakin	hyatt: review-	Details Formatted Diff Diff
Still not right (2.04 KB, patch) 2006-04-20 23:03 PDT, Beth Dakin	no flags	Details Formatted Diff Diff
I think this works... (3.25 KB, patch) 2006-04-20 23:52 PDT, Beth Dakin	hyatt: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alex Jones

Comment 1 2006-04-20 02:27:26 PDT

This problem is also showing up on digg.com (using the latest nightly).

Beth Dakin

Comment 2 2006-04-20 10:10:13 PDT

Created attachment 7851 [details] Reduction I have reduced this to: <head> <style> ul#nav li { background:url(http://i.a.cnn.net/cnn/.element/img/1.3/nav/nav.blue.gif) no-repeat; } </style> </head> <body> <ul id="nav"> <li></li> </ul> </body> If you look at the actual image that is used here, it is clear that we are squishing the whole thing to fit in the list marker, whereas before we would just display anything that fit.

Beth Dakin

Comment 3 2006-04-20 22:06:44 PDT

Created attachment 7863 [details] Patch This patch fixes the regression and maintains the correct behavior for backgrounds with both background-size and no-repeat set. It passes all of the layout tests.

Dave Hyatt

Comment 4 2006-04-20 22:46:55 PDT

Comment on attachment 7863 [details] Patch For now I recommend we remove the new NO_REPEAT clause you added. I think we need way more tests in the tree to baseline against before we stop calling drawTiledImage in the NO_REPEAT case. I noticed scaledWidth and scaledHeight initialized to bogus values earlier in the code. You can remove those and just declare scaledWidth/Height right above your code that sets them to the right values. There's a bug on the Mac only in tileInRect where oneTileRect is being used to set the fromRect's size and destination should be used instead.

Dave Hyatt

Comment 5 2006-04-20 22:55:58 PDT

Disregard my comment about adjusting scaledWidth/Height. Beth explained how that worked on IRC, and I was wrong.

Beth Dakin

Comment 6 2006-04-20 23:03:31 PDT

Created attachment 7864 [details] Still not right Can't figure out what is going wrong here...

Dave Hyatt

Comment 7 2006-04-20 23:16:17 PDT

Comment on attachment 7864 [details] Still not right This is right enough to fix the regressions though, and the tests that break don't have correct results even before the patch. We can land this and then file a followup bug about the failing test cases.

Beth Dakin

Comment 8 2006-04-20 23:52:27 PDT

Created attachment 7866 [details] I think this works... Even though the other patch was review+ed, i think this one takes care of the background-size regressions that one would cause...

Dave Hyatt

Comment 9 2006-04-21 00:01:06 PDT

Comment on attachment 7866 [details] I think this works... Yes. That's great. You should make a background-origin/size combined test.

Beth Dakin

Comment 10 2006-04-21 11:10:10 PDT

I committed the fix and added a few layout tests.

Darin Adler

Comment 11 2006-04-22 00:07:51 PDT

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

Darin Adler

Comment 12 2006-04-22 00:08:14 PDT

*** Bug 8508 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.