445 bytes, text/html
889 bytes, patch
|Details | Diff | Splinter Review|
1.48 KB, patch
Joe Drew (not getting mail): review+
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 Build ID: 20121119183901 Steps to reproduce: Go to the Bing Maps v7 Interactive SDK: http://www.bingmapsportal.com/isdk/ajaxv7#Pushpins9 (Pushpin attach click event is selected) Actual results: Clicked on the pushpin. Nothing happens. Click to the right of the pushpin and the click event is fired (in alert section box to the right you'll see text appear). Expected results: Clicking on the pushpin should fire the click event. Confirmed that Firefox 16 does not have this issue.
Created attachment 685412 [details] sample html I think that the problem is that the handling of the hotspot coordinates of the cur file is different between Firefox16 and Firefox17+. cursor: url("http://ecn.dev.virtualearth.net/mapcontrol/v7.0/7.0.20121012100453.93/cursors/grab.cur"), move; Regression window Good: http://hg.mozilla.org/mozilla-central/rev/4d59eb5ac2c6 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120822022846 Bad: http://hg.mozilla.org/mozilla-central/rev/88e47f6905e9 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120822090350 Pshlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4d59eb5ac2c6&tochange=88e47f6905e9 Regressed by: Bug 579517
At a guess, it's these changes in nsICODecoder::SetHotSpotIfCursor that broke things: nsCOMPtr<nsISupportsPRUint32> intwrapx = - do_CreateInstance("@mozilla.org/supports-PRUint32;1"); + do_CreateInstance("@mozilla.org/supports-uint32_t;1"); nsCOMPtr<nsISupportsPRUint32> intwrapy = - do_CreateInstance("@mozilla.org/supports-PRUint32;1"); + do_CreateInstance("@mozilla.org/supports-uint32_t;1"); Ehsan, can you take a look?
Why the interface name kept to nsISupportsPRUint32 while the contract ID has been changed to "@mozilla.org/supports-uint32_t;1"? At least they should be consistent.
The contract ID didn't change, except in that one method. That's the point.
Created attachment 685920 [details] [diff] [review] Patch (v1)
Comment on attachment 685920 [details] [diff] [review] Patch (v1) r=me, but any way we can add a testcase?
Comment on attachment 685920 [details] [diff] [review] Patch (v1) [Approval Request Comment] Regression caused by (bug #): bug 579517 User impact if declined: this will break clicking on regions with a custom .ico cursor. Testing completed (on m-c, etc.): locally. Risk to taking this patch (and alternatives if risky): as small as it gets!
oh good lord
(In reply to comment #7) > r=me, but any way we can add a testcase? Do you have any idea how we can test this? I'm not sure I do.
we'll take this low-risk fix in the 17.0.1 going to build tomorrow so that we reduce our shipping of this regression (only with our currently throttled early 17.0 users). Please land to esr17 relbranch and default.
Comment on attachment 685929 [details] [diff] [review] Test there is a world in which I would r- this for being in the wrong file, but I am not sure this is that world
https://hg.mozilla.org/releases/mozilla-esr17/rev/ed0f6806d1ef (relbranch) https://hg.mozilla.org/releases/mozilla-esr17/rev/a4cac71b5910
Comment on attachment 685929 [details] [diff] [review] Test https://hg.mozilla.org/integration/mozilla-inbound/rev/109e402b0b24
Verified on Firefox 19 RC, build ID: 20130215130331. Also verified on Firefox 17.0.3 ESR, build ID: 20130215125822. After clicking on the pushpin, the click event is fired.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 Build ID: 20130220104816 Verified as fixed on Firefox 20 beta 1 - clicking on the pushpin fires the click event.
mass remove verifyme requests greater than 4 months old