Bug 32078 - REGRESSION: Home photo slider is too narrow at http://www.ziprealty.com/
Summary: REGRESSION: Home photo slider is too narrow at http://www.ziprealty.com/
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Evan Martin
URL: http://www.ziprealty.com/
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2009-12-02 11:15 PST by Adele Peterson
Modified: 2010-10-17 14:01 PDT (History)
4 users (show)

See Also:


Attachments
beginnings of a reduction (170.74 KB, text/html)
2009-12-02 11:15 PST, Adele Peterson
no flags Details
patch (14.64 KB, patch)
2009-12-13 14:00 PST, Adele Peterson
sam: review+
Details | Formatted Diff | Diff
More reduced testcase (260.78 KB, text/html)
2009-12-14 13:44 PST, Simon Fraser (smfr)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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 1 Adele Peterson 2009-12-02 11:16:08 PST
<rdar://problem/7382815>
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
Any update?
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 11 Simon Fraser (smfr) 2009-12-13 16:10:37 PST
I also reopened bug 20674 :(
Comment 12 Adele Peterson 2009-12-13 23:02:40 PST
Also committed revision 52081.
Comment 13 Adele Peterson 2009-12-14 01:04:04 PST
Also committed r52083
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.
Comment 21 Simon Fraser (smfr) 2010-10-17 14:01:49 PDT
The progression that caused this bug was re-committed in http://trac.webkit.org/changeset/69928, but the site has changed so there's no need for a workaround.