Bug 23990 - Regression (r40837): JavaScript image popup doesn't work
Summary: Regression (r40837): JavaScript image popup doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P1 Normal
Assignee: Sam Weinig
URL: http://www.tetongravity.com/forums/sh...
Keywords: Regression
: 24134 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-02-17 08:25 PST by Alexander Kahn
Modified: 2009-02-25 23:18 PST (History)
4 users (show)

See Also:


Attachments
Reduced test case (7.36 KB, text/html)
2009-02-18 14:36 PST, Cameron Zwarich (cpst)
no flags Details
patch (5.33 KB, patch)
2009-02-24 18:22 PST, Sam Weinig
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kahn 2009-02-17 08:25:12 PST
Hi,

On this page: http://www.tetongravity.com/forums/showthread.php?t=137052&page=196 , in post #4877, clicking on one of the attached images is supposed to popup a larger version of the image. In the current nightly (41018?), it doesn't. 

However, when viewing the post on its own (http://www.tetongravity.com/forums/showpost.php?p=2260986&postcount=4877), the image pops up correctly.

Thanks to cpst for helping me file this.
Comment 1 Cameron Zwarich (cpst) 2009-02-17 08:43:07 PST
Using nightlies I tracked down this regression to the range r40813-r40884. I'll try bisecting some more with SVN.
Comment 2 Cameron Zwarich (cpst) 2009-02-17 10:40:26 PST
This regressed in r40837:

http://trac.webkit.org/changeset/40837

I'm not sure what caused the change.
Comment 3 Cameron Zwarich (cpst) 2009-02-18 10:49:30 PST
The YAHOO.util.Dom.setXY YUI function seems to be broken after r40837, but I am not sure why.
Comment 4 Cameron Zwarich (cpst) 2009-02-18 13:02:28 PST
The problem seems to be with the getXY YUI function, which sniffs getBoundingClientRect and uses it if it exists. In the case of the element in question, getBoundingClientRect().top differs from offsetTop.
Comment 5 Cameron Zwarich (cpst) 2009-02-18 13:59:34 PST
The problem is that getBoundingClientRect on all other browsers gives a negative top value if the page has been scrolled downwards. We need to emulate this behaviour (and other quirks of getBoundingClientRect) for compatibility.

I'll try to make a simple test case from this example.
Comment 6 Cameron Zwarich (cpst) 2009-02-18 14:36:51 PST
Created attachment 27764 [details]
Reduced test case

Here's a reduced test case. Clicking anywhere gives an alert with the top of the enclosing div's current bounding rectangle. Note that this is reported relative to the viewport in all other browsers.
Comment 7 Cameron Zwarich (cpst) 2009-02-18 16:06:07 PST
I'm unassigning this, because I'm probably not the one to fix it.
Comment 8 Kevin M. Dean 2009-02-24 07:57:57 PST
You mentioned scrolling the page downwards. I think I'm seeing what may be other related issues with scrolling between builds r40813 and r40884, so it could be the same bug.

I'm seeing that javascript opened content is only positioned correctly before you start scrolling. Once scrolled it opens in it's original position on the screen but not correct relative to the page content anymore.

Example: http://www.mania.com/aodvb/

On this and other forum sites, click the search or quick links drop down menus and they'll appear fine. Scroll down the page a little and then they appear out of proper position.

I'm also seeing something similar on Yahoo Personals in the photo gallery grid mode where an info box appears relative to a picture, but isn't in the right spot after scrolling.

Comment 9 Cameron Zwarich (cpst) 2009-02-24 09:02:36 PST
It's probably the same bug. The fix is to just use viewport-relative coordinates. If Sam doesn't fix it soon, I'll ask Hyatt how to fix it and do it myself.
Comment 10 Sam Weinig 2009-02-24 18:22:47 PST
Created attachment 27948 [details]
patch
Comment 11 Dave Hyatt 2009-02-24 18:36:50 PST
Comment on attachment 27948 [details]
patch

r=me
Comment 12 Sam Weinig 2009-02-24 18:38:52 PST
Fixed in r41207.
Comment 13 Cameron Zwarich (cpst) 2009-02-25 23:18:04 PST
*** Bug 24134 has been marked as a duplicate of this bug. ***