Closed
Bug 580576
Opened 14 years ago
Closed 14 years ago
Clicking "Share your location" in Fennec fails with an error in GeolocationPrompt.js
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: vingtetun, Assigned: vingtetun)
References
Details
Attachments
(3 files, 1 obsolete file)
978 bytes,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
5.67 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
5.85 KB,
patch
|
Details | Diff | Splinter Review |
Services.prefs does not provide the nsIContentPrefService interface. I wonder if this should be exposed in Services.jsm but with an other name? Gavin? In the worst case we can still use the old Cc["@mozilla.org/content-pref/service;1"].getService(Ci.nsIContentPrefService); call to keep it working.
Attachment #458974 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 1•14 years ago
|
||
This bug add a new .contentPrefs property to Services.jsm which map to nsIContentPrefService
Assignee: nobody → 21
Attachment #458975 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 2•14 years ago
|
||
Attachment #458974 -
Attachment is obsolete: true
Attachment #458996 -
Flags: review?(mark.finkle)
Attachment #458974 -
Flags: review?(mark.finkle)
Comment 3•14 years ago
|
||
Comment on attachment 458996 [details] [diff] [review] Patch v0.2 For Fennec >diff -r c562f5dc7354 components/GeolocationPrompt.js >+ prompt: function(aRequest) { > if (result == Ci.nsIPermissionManager.ALLOW_ACTION) { >- request.allow(); >+ aRequest.allow(); > return; > } >- if (result == Ci.nsIPermissionManager.DENY_ACTION) { >- request.cancel(); >+ else if (result == Ci.nsIPermissionManager.DENY_ACTION) { >+ aRequest.cancel(); if () { } else if { } cuddle! > function setPagePermission(uri, allow) { aURi and aAllow ? >- var prefService = Services.prefs; >- >- if (! prefService.hasPref(request.requestingURI, "geo.request.remember")) >- prefService.setPref(request.requestingURI, "geo.request.remember", 0); >- >- var count = prefService.getPref(request.requestingURI, "geo.request.remember"); >- >+ let contentPrefService = Services.contentPrefs; contentPrefs ? >+ let notificationBox = null; >+ if (aRequest.requestingWindow) { Can requestingWindow ever not be null? Maybe if the window is a chrome window. But if it is a chrome window, we don't need getChromeWindow anymore. >+ let requestingWindow = aRequest.requestingWindow.top; >+ let chromeWin = getChromeWindow(requestingWindow).wrappedJSObject; >+ notificationBox = chromeWin.getNotificationBox(requestingWindow); >+ } >+ else { cuddle the else, if we even need the if/else r+, but I'd like to figure out the requestingWindow issue. can be a followup
Attachment #458996 -
Flags: review?(mark.finkle) → review+
Comment 4•14 years ago
|
||
Comment on attachment 458975 [details] [diff] [review] Patch for toolkit Add a test for it to browser_Services.js ? r=me with that.
Attachment #458975 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 5•14 years ago
|
||
(In reply to comment #3) > Comment on attachment 458996 [details] [diff] [review] > Patch v0.2 For Fennec > >+ let notificationBox = null; > >+ if (aRequest.requestingWindow) { > > Can requestingWindow ever not be null? Maybe if the window is a chrome window. > But if it is a chrome window, we don't need getChromeWindow anymore. This is more a question of requestingElement being null for non-e10s. I don't know if that's normal or not? So requestingWindow can be normal DOMWindow from content (I'll check to be sure that's true)
Assignee | ||
Comment 6•14 years ago
|
||
This patch address comments
Assignee | ||
Comment 7•14 years ago
|
||
http://hg.mozilla.org/mobile-browser/rev/51cafd713607
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•