531008 - nsIDOMGeoPositionAddress must use DOMString instead of string

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Doug Turner (:dougt)]

i am aware that this changes an interface, but I am not sure sure of a better solution.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 414469 [details] [diff] [review]
patch v.1

Sigh, I guess this does have to block.

Doug, alternatives are:

 - add a new interface
 - don't change the UUID

This is a new 3.6 interface, so we shouldn't expect too much add-on churn here. Might even be able to just email all the authors of geolocation add-ons and notify them.

Comment 4

[Doug Turner (:dougt)]

i would rather just not change the UUID, and change the type.

And, I agree.  Most users are webcontent or in chrome/js and will not see this change.

Comment 5

[Johnny Stenback (:jst)]

There's a "binary compatible" alternative to changing the type to DOMString, we could change the type to utf8string and unicode data would make it through yet the C++ type would remain char*. That would still somewhat change the contract and expectations of C++ users of this, but I think the pain would be less that way than fully changing the type.

I think on trunk we should still change to using DOMString to avoid conversions and malloc overhead etc, but 1.9.2 we could consider using utf8string.

Comment 6

[Johnny Stenback (:jst)]

And yeah, blocking.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 414469 [details] [diff] [review]
patch v.1

r=jst to make this change on the trunk.

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 414469 [details] [diff] [review]
patch v.1

I think we should take this on 1.9.2.

Comment 9

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/ba9a490a6ef8

Comment 10

[Johnny Stenback (:jst)]

I'd be fine with making this change for 1.9.2 but I do think we should support the old interface there as well. IOW, rename the old interface to obsolete somethingorother, add the same interface we have on the trunk after this change, and make the implementations we have support both, which if they're in JS should be trivial (i.e. just make sure both interfaces are supported in QI).

Comment 11

[Doug Turner (:dougt)]

Attachment: patch v.1 - 1.9.2 patch

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 414681 [details] [diff] [review]
patch v.1 - 1.9.2 patch

r=bzbarsky

No need for approval for a blocker.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7d5d573dd500

Comment 14

[Eric Shepherd [:sheppy]]

Doc updated: https://developer.mozilla.org/en/nsIDOMGeoPositionAddress

Comment 15

[mitsugu oyama]

Can you see Bug 532535? I think the relation between this bug and Bug 532535. However, I think that discussing it by "Bug 532535" is appropriate.