Bug 8495 - REGRESSION: Sidebar on cnn.com is hosed
Summary: REGRESSION: Sidebar on cnn.com is hosed
Status: RESOLVED FIXED
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.cnn.com/SHOWBIZ/
Keywords:
: 8508 8518 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-04-19 22:49 PDT by Dave Hyatt
Modified: 2006-04-22 00:08 PDT (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 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.
Comment 1 Alex Jones 2006-04-20 02:27:26 PDT
This problem is also showing up on digg.com (using the latest nightly).
Comment 2 Beth Dakin 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.
Comment 3 Beth Dakin 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.
Comment 4 Dave Hyatt 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.
Comment 5 Dave Hyatt 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.
Comment 6 Beth Dakin 2006-04-20 23:03:31 PDT
Created attachment 7864 [details]
Still not right

Can't figure out what is going wrong here...
Comment 7 Dave Hyatt 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.
Comment 8 Beth Dakin 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...
Comment 9 Dave Hyatt 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.
Comment 10 Beth Dakin 2006-04-21 11:10:10 PDT
I committed the fix and added a few layout tests.
Comment 11 Darin Adler 2006-04-22 00:07:51 PDT
*** Bug 8518 has been marked as a duplicate of this bug. ***
Comment 12 Darin Adler 2006-04-22 00:08:14 PDT
*** Bug 8508 has been marked as a duplicate of this bug. ***