Last Comment Bug 531008 - nsIDOMGeoPositionAddress must use DOMString instead of string
: nsIDOMGeoPositionAddress must use DOMString instead of string
: dev-doc-complete
Product: Core
Classification: Components
Component: Geolocation (show other bugs)
: unspecified
: All All
: -- blocker (vote)
: ---
Assigned To: Doug Turner (:dougt)
: Andrew Overholt [:overholt]
: 532535 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2009-11-25 00:23 PST by Doug Turner (:dougt)
Modified: 2010-08-26 15:25 PDT (History)
5 users (show)
jst: blocking1.9.2+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v.1 (1.34 KB, patch)
2009-11-25 00:23 PST, Doug Turner (:dougt)
jst: review+
Details | Diff | Splinter Review
patch v.1 - 1.9.2 patch (2.42 KB, patch)
2009-11-25 20:12 PST, Doug Turner (:dougt)
bzbarsky: review+
Details | Diff | Splinter Review

Description Doug Turner (:dougt) 2009-11-25 00:23:04 PST

Comment 1 Doug Turner (:dougt) 2009-11-25 00:23:30 PST
Created attachment 414469 [details] [diff] [review]
patch v.1
Comment 2 Doug Turner (:dougt) 2009-11-25 00:33:39 PST
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] 2009-11-25 08:45:01 PST
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) 2009-11-25 08:56:16 PST
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, 2009-11-25 10:47:15 PST
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, 2009-11-25 10:48:34 PST
And yeah, blocking.
Comment 7 Johnny Stenback (:jst, 2009-11-25 10:51:01 PST
Comment on attachment 414469 [details] [diff] [review]
patch v.1

r=jst to make this change on the trunk.
Comment 8 Doug Turner (:dougt) 2009-11-25 13:18:44 PST
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) 2009-11-25 13:18:51 PST
Comment 10 Johnny Stenback (:jst, 2009-11-25 14:53:42 PST
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) 2009-11-25 20:12:41 PST
Created attachment 414681 [details] [diff] [review]
patch v.1 - 1.9.2 patch
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2009-11-30 16:21:31 PST
Comment on attachment 414681 [details] [diff] [review]
patch v.1 - 1.9.2 patch


No need for approval for a blocker.
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2009-11-30 16:24:29 PST
Comment 14 Eric Shepherd [:sheppy] 2009-12-17 22:19:58 PST
Doc updated:
Comment 15 mitsugu oyama 2009-12-18 03:27:21 PST
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.
Comment 16 Doug Turner (:dougt) 2010-08-26 15:25:57 PDT
*** Bug 532535 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.