580576 - Clicking "Share your location" in Fennec fails with an error in GeolocationPrompt.js

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch for Fennec

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.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch for toolkit

This bug add a new .contentPrefs property to Services.jsm which map to nsIContentPrefService

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2 For Fennec

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 458975 [details] [diff] [review]
Patch for toolkit

Add a test for it to browser_Services.js ? r=me with that.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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)

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.3 for checking

This patch address comments

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/51cafd713607