nsIDOMGeoPositionAddress must use DOMString instead of string

RESOLVED FIXED

Status

()

Core
Geolocation
--
blocker
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

({dev-doc-complete})

unspecified
dev-doc-complete
Points:
---
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta5-fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

8 years ago
Created attachment 414469 [details] [diff] [review]
patch v.1
Assignee: nobody → mozbugz
(Assignee)

Updated

8 years ago
Flags: blocking1.9.2?
(Assignee)

Updated

8 years ago
Attachment #414469 - Flags: review?(jst)
(Assignee)

Comment 2

8 years ago
i am aware that this changes an interface, but I am not sure sure of a better solution.
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.
(Assignee)

Comment 4

8 years ago
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.
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.
And yeah, blocking.
Flags: blocking1.9.2? → blocking1.9.2+

Updated

8 years ago
Attachment #414469 - Flags: review?(jst) → review+
Comment on attachment 414469 [details] [diff] [review]
patch v.1

r=jst to make this change on the trunk.
(Assignee)

Comment 8

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

I think we should take this on 1.9.2.
Attachment #414469 - Flags: approval1.9.2?
(Assignee)

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/ba9a490a6ef8
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).
(Assignee)

Comment 11

8 years ago
Created attachment 414681 [details] [diff] [review]
patch v.1 - 1.9.2 patch
Attachment #414681 - Flags: review?(jst)
Attachment #414681 - Flags: approval1.9.2?
(Assignee)

Updated

8 years ago
Attachment #414469 - Flags: approval1.9.2?

Comment 12

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

r=bzbarsky

No need for approval for a blocker.
Attachment #414681 - Flags: review?(jst)
Attachment #414681 - Flags: review+
Attachment #414681 - Flags: approval1.9.2?

Comment 13

8 years ago
Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7d5d573dd500
Status: NEW → RESOLVED
Last Resolved: 8 years ago
status1.9.2: --- → final-fixed
Resolution: --- → FIXED
Doc updated: https://developer.mozilla.org/en/nsIDOMGeoPositionAddress
Keywords: dev-doc-complete

Comment 15

8 years ago
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.
(Assignee)

Updated

7 years ago
Duplicate of this bug: 532535
You need to log in before you can comment on or make changes to this bug.