|Summary:||REGRESSION: Home photo slider is too narrow at http://www.ziprealty.com/|
|Product:||WebKit||Reporter:||Adele Peterson <adele>|
|Component:||CSS||Assignee:||Evan Martin <evan>|
|Severity:||Normal||CC:||bdakin, evan, simon.fraser, webkit.review.bot|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Adele Peterson 2009-12-02 11:15:27 PST
Created attachment 44165 [details] beginnings of a reduction Steps to reproduce 1. Go to http://www.ziprealty.com/ 2. Under the "Search Featured Homes" section, there should be a home photo slider ( with left/right navigation arrows on both sides of the slider) 3. The slider is functional but is very narrow. This was caused by http://trac.webkit.org/changeset/49585, which was part of fixing https://bugs.webkit.org/show_bug.cgi?id=18994 Attaching the beginnings of a reduction, with an alert for debugging. The problem is related to the fact that marginRight for the <ul> element is -9999970px
Comment 2 Evan Martin 2009-12-02 11:25:04 PST
Tears. I would guess that the problem is related to that I changed the range of valid values, limiting the range. However, the test includes verifying that the number "98765432198" makes it through ok. I recall that some of those tests may not pass on some platforms, but I don't see any platform-specific failures on these tests. I may be looking in the wrong place though? (The tests to look at are fast/css/opacity-float and fast/css/large-number-round-trip.) I am sorry for the trouble! I am not opposed to a revert of my change, as it is fixing a relatively obscure corner of the code, but I'd like to understand what went wrong.
Comment 3 Adele Peterson 2009-12-02 11:27:34 PST
I don't think we need to revert right now. Its fine to investigate first.
Comment 4 Adele Peterson 2009-12-11 17:02:22 PST
Comment 5 Evan Martin 2009-12-12 09:56:49 PST
Oh, sorry! I thought you were looking at it. I am going to be travelling for the next few weeks but I hopefully can find some time for it.
Comment 6 Adele Peterson 2009-12-12 13:54:44 PST
I should have been clearer. I assumed you were going to fix it since you wrote the original change. Let's roll out r49585 for now, and you can submit a new patch once you've figured out what the issue is.
Comment 7 Adele Peterson 2009-12-13 14:00:03 PST
Created attachment 44762 [details] patch I'll reopen the other bug when this is committed. One thing I noticed is that the new tests that were added to opacity-float.html didn't pass in Firefox. I wonder if the JS library used at this site was just expecting wrong behavior. Might be something investigate when re-fixing the bug.
Comment 8 WebKit Review Bot 2009-12-13 14:05:14 PST
style-queue ran check-webkit-style on attachment 44762 [details] without any errors.
Comment 9 Sam Weinig 2009-12-13 14:52:13 PST
Comment on attachment 44762 [details] patch This is missing a changelog, but otherwise is fine. r=me
Comment 10 Adele Peterson 2009-12-13 15:00:38 PST
Committed revision 52071.
Comment 12 Adele Peterson 2009-12-13 23:02:40 PST
Also committed revision 52081.
Comment 14 Simon Fraser (smfr) 2009-12-14 13:44:23 PST
Created attachment 44820 [details] More reduced testcase This testcase runs in Firefox as well, which has the same bug (numbers in scientific notation fail to parse). So I think the site is relying on broken behavior.
Comment 15 Adele Peterson 2009-12-14 13:48:31 PST
Why doesn't Firefox hit the bug on the live site? Spoofing as Firefox doesn't work, so I wonder what kind of detection the site is doing.
Comment 16 Simon Fraser (smfr) 2009-12-14 13:54:57 PST
Firefox has the same behavior as WebKit did before bug 18994 was fixed. So the site is relying on broken behavior in WebKit and Firefox. I don't think we want to hold back fixing the underlying behavior for this one site.
Comment 17 Adele Peterson 2009-12-14 22:20:25 PST
hmm ok. We should consider a site-specific hack (not sure what the threshold for that should be), and we should have an evangelism bug for this too.
Comment 18 Adele Peterson 2009-12-15 14:51:22 PST
Do any browsers have the right behavior for this?
Comment 19 Simon Fraser (smfr) 2009-12-15 15:00:27 PST
Opera (Mac) seems to read the width as zero, and then clamp it to 32767px. The page ends up rendering correctly.
Comment 20 Simon Fraser (smfr) 2009-12-15 15:47:35 PST
The testcase doesn't run in IE8, but the CSS inspector shows the width of the element to be 1342177.27px. The page renders correctly.