Closed Bug 815359 Opened 7 years ago Closed 7 years ago

Bing Maps v7 Ajax Pushpin Incompatibility

Categories

(Core :: ImageLib, defect)

17 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
firefox17 + fixed
firefox18 + fixed
firefox19 + verified
firefox20 + verified
firefox-esr17 + verified

People

(Reporter: dastclair, Assigned: ehsan)

References

Details

(Keywords: regression)

Attachments

(3 files)

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.
Attached file 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
Blocks: stdint
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
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?
Assignee: nobody → ehsan
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.
Holy cow!!!
Attached patch Patch (v1)Splinter Review
Attachment #685920 - Flags: review?(bzbarsky)
Comment on attachment 685920 [details] [diff] [review]
Patch (v1)

r=me, but any way we can add a testcase?
Attachment #685920 - Flags: review?(bzbarsky) → review+
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!
Attachment #685920 - Flags: approval-mozilla-release?
Attachment #685920 - Flags: approval-mozilla-esr17?
Attachment #685920 - Flags: approval-mozilla-beta?
Attachment #685920 - Flags: approval-mozilla-aurora?
Component: General → ImageLib
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.
Attached patch TestSplinter Review
Attachment #685929 - Flags: review?(joe)
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
Attachment #685929 - Flags: review?(joe) → review+
Attachment #685920 - Flags: approval-mozilla-release?
Attachment #685920 - Flags: approval-mozilla-release+
Attachment #685920 - Flags: approval-mozilla-esr17?
Attachment #685920 - Flags: approval-mozilla-esr17+
Attachment #685920 - Flags: approval-mozilla-beta?
Attachment #685920 - Flags: approval-mozilla-beta+
Attachment #685920 - Flags: approval-mozilla-aurora?
Attachment #685920 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/a2ee1d41edff
https://hg.mozilla.org/mozilla-central/rev/109e402b0b24
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
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.
QA Contact: manuela.muntean
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
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.