Bug 10983 - REGRESSION (r12290): Drop shadow of flickr photo note is positioned incorrectly the second time it's shown
Summary: REGRESSION (r12290): Drop shadow of flickr photo note is positioned incorrect...
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: Nobody
URL: http://flickr.com/photos/babykailan/2...
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2006-09-22 01:59 PDT by mitz
Modified: 2007-01-26 03:55 PST (History)
1 user (show)

See Also:


Attachments
Partial reduction (1.59 KB, text/html)
2006-09-22 02:01 PDT, mitz
no flags Details
Root cause? (not a regression) (846 bytes, text/html)
2006-09-22 04:21 PDT, mitz
no flags Details
"Static position" test case (391 bytes, text/html)
2006-12-27 10:26 PST, mitz
no flags Details
Ensure proper relayout when the static y position changes. (14.65 KB, patch)
2007-01-24 15:16 PST, mitz
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2006-09-22 01:59:06 PDT
If you go to the URL and mouse over the photo and into the note square, the note shows up. The first time it looks ok, but on subsquent mouseovers the drop shadow is positioned a few pixels above where it should be.

The regression appears to be in r12290 (fix for bug 3508).
Comment 1 mitz 2006-09-22 02:01:05 PDT
Created attachment 10700 [details]
Partial reduction
Comment 2 mitz 2006-09-22 04:21:58 PDT
Created attachment 10703 [details]
Root cause? (not a regression)

The partial reduction reduced to this case, which renders incorrectly in shipping Safari too. In TOT, the positions are corrected at the next relayout, e.g. if you resize the window or mousedown on a button (even before releasing it), whereas in shipping Safari, hiding and showing will reproduce the bad layout. Looks like Flickr actually expects the bad layout.
Comment 3 Dave Hyatt 2006-09-22 12:36:38 PDT
So did Firefox match our rendering for 3508?
Comment 4 mitz 2006-09-22 12:49:37 PDT
(In reply to comment #3)
> So did Firefox match our rendering for 3508?
> 

No, Firefox matches TOT. Flickr possibly tries to work around the bug in shipping Safari (by "the bug" I don't mean bug 3508, but rather the fact that dynamically setting the margin results in incorrect layout). I think the bottom line is that TOT is broken, but shipping Safari is even more broken.
Comment 5 mitz 2006-09-24 06:45:24 PDT
Looks like Firefox, Opera and (normally) WebKit collapse the margins between a static box and an absolutely positioned box, which http://www.w3.org/TR/CSS21/box.html#collapsing-margins suggests that you shouldn't do (the spec doesn't make an exception for "absolutely positioned with top: auto"). WinIE doesn't do that.

I think the positioning of the note in http://flickr.com/photos/jhawke/231017499/in/set-72057594049560325/ (when you're not logged in to flickr, so there's no toolbar between the title and the photo) is wrong in Firefox and Opera. In WebKit it happens to be positioned correctly only because it's dynamic, but the drop shadow eventually moves up.
Comment 6 mitz 2006-09-24 22:57:50 PDT
Hyatt explained to me that the "collapsing" is correct, but it isn't really collapsing: with 'top' and 'bottom' being 'auto', you set the vertical margins to 'auto' and use the static position, according to case 2 of http://www.w3.org/TR/CSS21/visudet.html#abs-non-replaced-height .

Flickr will need to change their website to lay out correctly in Firefox and -- once this bug is fixed -- WebKit.
Comment 7 David Kilzer (:ddkilzer) 2006-09-26 14:52:45 PDT
(In reply to comment #6)
> Flickr will need to change their website to lay out correctly in Firefox and --
> once this bug is fixed -- WebKit.

Is a separate evangelism bug needed to track this?
Comment 8 mitz 2006-12-27 10:26:19 PST
Created attachment 12065 [details]
"Static position" test case

Here is a simple case, which has not changed between shipping WebKit and TOT.
Comment 9 mitz 2007-01-24 15:16:02 PST
Created attachment 12652 [details]
Ensure proper relayout when the static y position changes.

Includes a test for three cases fixed by this patch and another one that has been fixed but doesn't have a test.
Comment 10 Darin Adler 2007-01-24 16:19:55 PST
Comment on attachment 12652 [details]
Ensure proper relayout when the static y position changes.

Looks good to me, but I'm slightly out of my depth. Needs a layout expert review.
Comment 11 Maciej Stachowiak 2007-01-24 20:25:51 PST
Comment on attachment 12652 [details]
Ensure proper relayout when the static y position changes.

I'm sufficiently convinced by the test cases and the relatively simple changed, plus the fact that Mitz himself is a layout expert.
Comment 12 Mark Rowe (bdash) 2007-01-26 03:55:32 PST
Landed in r19148.