Refactor BrowserApp-based window type detection

RESOLVED FIXED in Firefox 57

Status

()

Firefox for Android
GeckoView
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: esawin, Assigned: esawin)

Tracking

51 Branch
Firefox 57
All
Android
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 months ago
In bug 1329154 we've added BrowserApp to the GeckoView window which breaks some modules that identify GeckoView windows by the absence of BrowserApp.
(Assignee)

Comment 1

2 months ago
Created attachment 8907650 [details] [diff] [review]
0001-Bug-1399518-1.0-Use-windowtype-to-detect-GeckoView-w.patch

The windowtype should be a reliable way to detect GeckoView windows.
Do we use this type of check anymore else?
Attachment #8907650 - Flags: review?(nchen)
Comment on attachment 8907650 [details] [diff] [review]
0001-Bug-1399518-1.0-Use-windowtype-to-detect-GeckoView-w.patch

Review of attachment 8907650 [details] [diff] [review]:
-----------------------------------------------------------------

What do you think about just checking for the feature? (e.g. `chromeWin.NativeWindow && chromeWin.NativeWindow.doorhanger`)

So if we ever have a `NativeWindow.doorhanger` implementation for GeckoView we would use that automatically.

::: mobile/android/modules/Prompt.jsm
@@ +23,5 @@
>              .rootTreeItem.QueryInterface(Ci.nsIInterfaceRequestor)
>              .getInterface(Ci.nsIDOMWindow);
>  }
>  
> +function isGeckoView(aWindow) {

This feels a little backwards. I think it should be `isBrowserJS`, and we should compare against "navigator:browser".
Attachment #8907650 - Flags: review?(nchen) → review+

Comment 3

2 months ago
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a35599c5d1e
[1.1] Check for NativeWindow features to identify the window type for prompt handling. r=jchen
(Assignee)

Comment 4

2 months ago
Created attachment 8907738 [details] [diff] [review]
0001-Bug-1399518-1.1-Check-for-NativeWindow-features-to-i.patch
Attachment #8907650 - Attachment is obsolete: true
Attachment #8907738 - Flags: review+

Comment 5

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3a35599c5d1e
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.