Closed
Bug 1365868
Opened 8 years ago
Closed 7 years ago
Support minimal context menu functionality in GeckoView-based custom tabs
Categories
(GeckoView :: General, enhancement, P1)
Tracking
(firefox57 verified)
VERIFIED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: JanH, Assigned: cnevinchen)
References
Details
(Whiteboard: [FNC][SPT57.3][INT])
Attachments
(1 file)
Things that work at the moment and won't work automatically after switching to GeckoView:
- context menus
- context menu entries added by add-ons
- "Open Link in New (Private) Tab" opens a new tab in Firefox and shows the usual snackbar - pressing the "Switch" button on that then switches to our normal BrowserApp-based UI to show that tab
- "Parent tab" functionality: Pressing the back button on a normal tab that was opened from a custom tab/web app closes that tab again and returns to the original custom tab/web app activity. For completeness, at the moment closing such a tab from the tabs tray also returns to the original activity, but we might want to rethink that particular bit of behaviour (compare bug 1363049).
Reporter | ||
Updated•8 years ago
|
Blocks: customtabs_geckoview, pwa_geckoview
No longer depends on: customtabs_geckoview, pwa_geckoview
Reporter | ||
Comment 1•8 years ago
|
||
(In reply to Jan Henning [:JanH] from comment #0)
> - "Parent tab" functionality: Pressing the back button on a normal tab that
> was opened from a custom tab/web app closes that tab again and returns to
> the original custom tab/web app activity. For completeness, at the moment
> closing such a tab from the tabs tray also returns to the original activity,
> but we might want to rethink that particular bit of behaviour (compare bug
> 1363049).
Current state of things after bug 1363049 is now that we return to a web app/custom tab "parent tab" only when using the back button.
Updated•7 years ago
|
Component: General → GeckoView
Blocks: customtabs
We now have a context menu API in GeckoView. We can't hook this up to the Fennec context menu code, however, because that's all implemented in JS. I think we need someone to do it using regular 'ol Java. Nevin, can you guys do this?
Flags: needinfo?(cnevinchen)
Assignee | ||
Comment 3•7 years ago
|
||
Hi Joe, Wesly
Please help prioritize this.
Hi snorp
Do you know where I can find the GeckoView context menu API?
Thanks!
Flags: needinfo?(wehuang)
Flags: needinfo?(snorp)
Flags: needinfo?(jcheng)
Flags: needinfo?(cnevinchen)
Comment 4•7 years ago
|
||
Unless Joe has other say, I think we have reserved some time in this sprint for potential photon/Leanplum QA-bug, so we should be able to prioritize this after you complete your MVP and the 2 important BL items.
Flags: needinfo?(wehuang)
CustomTabsActivity has an unimplemented handler ready to go here: https://dxr.mozilla.org/mozilla-central/rev/db7f19e26e571ae1dd309f5d2f387b06ba670c30/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java#600
The docs for that callback are here: https://dxr.mozilla.org/mozilla-central/rev/db7f19e26e571ae1dd309f5d2f387b06ba670c30/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoView.java#1355
Note that currently all GeckoView callbacks are executed on the Gecko thread.
Flags: needinfo?(snorp)
Assignee | ||
Comment 7•7 years ago
|
||
Below are data I receive in the callback. I'll need to check if below information is enough for showing the corresponding context menus.
Long Click on Image:
view = {GeckoView@5813} "org.mozilla.gecko.GeckoView{d32c57a VFE...... .F...... 0,147-1080,1731 #7f0f00c4 app:id/gecko_view}"
screenX = 158
screenY = 93
uri = null
elementSrc = "https://www.google.com.tw/images/branding/searchlogo/1x/googlelogo_tablet_tier1_hp_color_183x64dp.png"
Long Click on Youtube:
view = ......
screenX = 115
screenY = 205
uri = "https://m.youtube.com/watch?v=h4s0llOpKrU"
elementSrc = null
Long Click on URL Link:
view = .....
screenX = 118
screenY = 274
uri = "http://m.dior.com/zh_tw/minisite/th/miss_dior.html?utm_campaign=missdior_tw_sep17&utm_content=hp_masthead_displaymobile&utm_medium=display&utm_source=youtube"
elementSrc = null
Assignee | ||
Comment 8•7 years ago
|
||
Below is what I got at PromptServie[1]
We need to port JS implementation[2]~[3] and [4]~[5] to Java.
Moving from browser.js to Java is a great approach but I can't complete this in my current time frame.
D: handleMessage() called with: geckoBundle = [{showAsActions={uri=http://www.rapidtables.com/web/html/mailto.htm#how, title=How to create mailto link in HTML?}, id=6, icon=drawable://ic_menu_share, label=Share Link}]
D: handleMessage() called with: geckoBundle = [{id=1, label=Open Link in New Tab}]
D: handleMessage() called with: geckoBundle = [{id=2, label=Open Link in Private Tab}]
D: handleMessage() called with: geckoBundle = [{id=3, label=Copy Link}]
D: handleMessage() called with: geckoBundle = [{id=11, label=Bookmark Link}]
D: handleMessage() called with: event = [Prompt:Show], message = [{listitems=[Lorg.mozilla.gecko.util.GeckoBundle;@4da2c8, type=null, async=true, tabId=0, title=http://www.rapidtables.com/web/html/mailto.htm#how}], callback = [org.mozilla.gecko.EventDispatcher$NativeCallbackDelegate@7576f61]
D: handleMessage() called with: geckoBundle = [{showAsActions={uri=https://m.youtube.com/watch?v=cd3HriqJH98, title=0:36}, id=6, icon=drawable://ic_menu_share, label=Share Link}]
D: handleMessage() called with: geckoBundle = [{id=1, label=Open Link in New Tab}]
D: handleMessage() called with: geckoBundle = [{id=2, label=Open Link in Private Tab}]
D: handleMessage() called with: geckoBundle = [{id=3, label=Copy Link}]
D: handleMessage() called with: geckoBundle = [{id=11, label=Bookmark Link}]
D: handleMessage() called with: geckoBundle = [{id=26, label=Open With YouTube App}]
D: handleMessage() called with: event = [Prompt:Show], message = [{listitems=[Lorg.mozilla.gecko.util.GeckoBundle;@375258a, type=null, async=true, tabId=0, title=https://m.youtube.com/watch?v=cd3HriqJH98}], callback = [org.mozilla.gecko.EventDispatcher$NativeCallbackDelegate@d0d37fb]
[1] https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/prompts/PromptService.java#42
initialize
[2] https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#582
[3] https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#976
logic
[4] https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#2413
[5] https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3112
Flags: needinfo?(wehuang)
Comment 9•7 years ago
|
||
(In reply to Wesly Huang (Fennec Frontend/Firefox EPM) from comment #4)
> Unless Joe has other say, I think we have reserved some time in this sprint
> for potential photon/Leanplum QA-bug, so we should be able to prioritize
> this after you complete your MVP and the 2 important BL items.
Yes Nevin please secure the Leanplum Push notification testing toward the pre-beta signoff first as agreed with Product, thanks.
Flags: needinfo?(wehuang)
No longer blocks: customtabs
Blocks: customtabs
Updated•7 years ago
|
Priority: -- → P2
Comment 11•7 years ago
|
||
Per discussion with team, for 57 MVP, we'd want to at least respond to long press on links/images with only one option (Open in "Firefox")
Flags: needinfo?(jcheng)
Priority: P2 → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
The "Open in XXX" here will open the target telement in Fennec (The browser APP). While you click "Oepein in XXX" on the menu, it will open the original page in Fennec.
Assignee: nobody → cnevinchen
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8905394 [details]
Bug 1365868 - Add basic context menu: Open in Firefox.
https://reviewboard.mozilla.org/r/177196/#review182662
::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:623
(Diff revision 2)
> + OpenInFennec(content, CustomTabsActivity.this);
> + }
> + });
> + }
> +
> + void OpenInFennec(final String content, final Context context) {
Do we prefer method name with uppercase 'O' in this class? or it's a kotlin style? :P
::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:633
(Diff revision 2)
> + public void onPromptFinished(final GeckoBundle result) {
> +
> + final int itemId = result.getInt("button", -1);
> +
> + if (itemId == -1) {
> +
Add a comment to explicit indicate this is the error case. or early return before below startActivity
::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:653
(Diff revision 2)
> +
> + }
> +
> + boolean isValidURL(String urlString) {
> + try {
> + URL url = new URL(urlString);
Why do we need to parse as URL and toURI, instead of URI.parseURI()?
Or is it better if we change `content` from String to URI and parse only once use this?
org.mozilla.gecko.util.URIUtils#uriOrNull
Attachment #8905394 -
Flags: review?(max) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8905394 [details]
Bug 1365868 - Add basic context menu: Open in Firefox.
https://reviewboard.mozilla.org/r/177196/#review182764
::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:611
(Diff revision 5)
>
> @Override
> public void onContextMenu(GeckoView view, int screenX, int screenY,
> - String uri, String elementSrc) {}
> + final String uri, final String elementSrc) {
> +
> + final String content = uri != null ? uri : elementSrc != null ? elementSrc : "";
Note: this would "hide" nested images and media elements (e.g, <a><img></a>).
Should we want to allow actions on the nested element, we will need to consider both URIs and offer some split view akin to Fennec.
Assignee | ||
Comment 20•7 years ago
|
||
Thanks Eugen. We should defintely consider a more complete scenario. This patch here is just for quick fix for 57. I've open bug 1398197 for it.
Assignee | ||
Comment 21•7 years ago
|
||
oops.. just found Jan had opened bug 1398067 for comment 20. We should do it there.
Summary: Support context menu and "tab" related functionality in GeckoView-based custom tabs/web apps → Support minimal context menu functionality in GeckoView-based custom tabs
Comment 22•7 years ago
|
||
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c9a3b09cfc1
Add basic context menu: Open in Firefox. r=maliu
Comment 23•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•7 years ago
|
Whiteboard: [FNC][SPT57.3][INT]
Comment 24•7 years ago
|
||
Verified as fixed in 57.0b9.
Devices: Motorola Nexus 6 (Android 7.1.1) and Sony Xperia Z5 Premium (Android 6.0.1).
"Open in Firefox Beta" message is displayed when long tap on a link opened in Custom Tab.
Comment 25•7 years ago
|
||
Is this bug fixed in Nightly builds? Because it's still reproducible. Long tap on a link opens selection menu.
Flags: needinfo?(cnevinchen)
(In reply to Sorina Florean [:sorina] from comment #25)
> Is this bug fixed in Nightly builds? Because it's still reproducible. Long
> tap on a link opens selection menu.
There's a GeckoView problem here, bug 1411529.
Flags: needinfo?(cnevinchen)
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox for Android → GeckoView
Updated•6 years ago
|
Target Milestone: Firefox 57 → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•