1347165 - Change context menu "open in new tab" to "open in Firefox tab" when not in browser

Status: RESOLVED DUPLICATE of bug 1365868

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P2)

Description

[Dale Harvey (:daleharvey)]

When I open a website in standalone mode, on long pressing links I should not see "open in new tab" in the context menu

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

In Custom Tabs Chrome recently added this:
http://www.androidpolice.com/2017/03/10/chrome-58-dev-adds-ability-open-custom-tab-links-new-chrome-tabs/

Maybe it would be useful in a web app too if it's something like "Open in new Firefox tab"?

Comment 2

[Dale Harvey (:daleharvey)]

Yup I was thinking along the same lines too, if anyone else wanted to pick this up and do it for webapps + custom tabs that would be great

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

ni? :jalin on UX input

Comment 4

[Jack Lin[:jacklin]]

(In reply to Sebastian Kaspari (:sebastian) from comment #1)
> In Custom Tabs Chrome recently added this:
> http://www.androidpolice.com/2017/03/10/chrome-58-dev-adds-ability-open-
> custom-tab-links-new-chrome-tabs/
> 
> Maybe it would be useful in a web app too if it's something like "Open in
> new Firefox tab"?

Agree. I think we can provide "Open in new Firefox tab" and "Open in new Firefox incognito tab" on context menu of webapps. 
 

(In reply to Dale Harvey (:daleharvey) from comment #2)
> Yup I was thinking along the same lines too, if anyone else wanted to pick
> this up and do it for webapps + custom tabs that would be great

I am wondering webapps developers can have custom tabs features like changing background color or customizing functions on action bar/menu? If yes, webapps + custom tabs would be great. Otherwise, I would prefer opening Firefox browser because users can have all browser functions.

Comment 5

[Dale Harvey (:daleharvey)]

> I am wondering webapps developers can have custom tabs features 
> like changing background color or customizing functions on action bar/menu?

Sorry I think there was a little confusion, I meant if we could change the context menu when we are within a webapp or custom tab to say "open in new Firefox tab" it would be great.

Comment 6

[Jack Lin[:jacklin]]

(In reply to Dale Harvey (:daleharvey) from comment #5)
> > I am wondering webapps developers can have custom tabs features 
> > like changing background color or customizing functions on action bar/menu?
> 
> Sorry I think there was a little confusion, I meant if we could change the
> context menu when we are within a webapp or custom tab to say "open in new
> Firefox tab" it would be great.

Got it :)
Agree with you.

Comment 7

[Dale Harvey (:daleharvey)]

So the only thing I am unsure about with this is how to have the context menu detect its inside standalone mode, there is https://developer.mozilla.org/en-US/docs/Web/CSS/@media/display-mode which would be idea, but may take a while to implement and not work for custom tabs?

Comment 8

[Dale Harvey (:daleharvey)]

Looking at the spec, it seems like custom tabs could be "minimal-ui" and we determine whether or not you are in the browser from if display-mode: browser, that sounds nice and clean but perhaps a lot of engineering

Comment 9

[Jan Henning [:JanH]]

Principally you could also use the tab type from bug 1346004, although I've no idea what's the better solution in the end...

Comment 10

[Jan Henning [:JanH]]

That description reads as if any content displayed in fullscreen mode might end up with the corresponding diyplay mode, though, in which case how would one differentiate the context menu between a fullscreen browsing tab, web app and custom tab?

Comment 11

[Dale Harvey (:daleharvey)]

Hmm yup you are right about that I dont think there is a distinction, will look into exposing the tab types somehow, maybe just a message when they change? browser.js maintains its own list of tabs already

Comment 12

[Jan Henning [:JanH]]

Changing tab types after their creation would be bug 1348686.

Comment 14

[Jan Henning [:JanH]]

At the moment, opening a link in a new tab already opens a normal browsing-type tab, which will be displayed in Firefox's tab list. If we also manage to get bug 1351739 going, that would also take care of the snackbar for new tabs with the "Switch" button - clicking it selects the new tab, which in turn would then switch to the BrowserApp UI.

As for this bug, at the moment context menu initialisation is fire-and-forget (https://dxr.mozilla.org/mozilla-central/rev/272ce6c2572164f5f6a9fba2a980ba9ccf50770c/mobile/android/chrome/content/browser.js#604).
We'd have to change this such as to remember the menu id of the "open in new (private) tab" menu entries and then each time
- a tab is selected
- a tab changes its type (after bug 1348686)

we check the type of the currently selected tab and if it's changed, we need to update (https://dxr.mozilla.org/mozilla-central/rev/272ce6c2572164f5f6a9fba2a980ba9ccf50770c/mobile/android/chrome/content/browser.js#2281, https://dxr.mozilla.org/mozilla-central/rev/272ce6c2572164f5f6a9fba2a980ba9ccf50770c/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#3216) the text for the context menu entry.

Comment 15

[Jan Henning [:JanH]]

(In reply to Jan Henning [:JanH] from comment #14)
> we check the type of the currently selected tab

https://dxr.mozilla.org/mozilla-central/rev/272ce6c2572164f5f6a9fba2a980ba9ccf50770c/mobile/android/chrome/content/browser.js#3567

Comment 16

[Jan Henning [:JanH]]

Alternatively, since we already pass a dynamic string getter to Nativewindow for handling locale changes, we could extend that for those two menu items and pass a special function that also takes the current tab type into account.

Comment 17

[Dale Harvey (:daleharvey)]

> As for this bug, at the moment context menu initialisation is fire-and-forget ....

I took a quick look at this and instead of tracking it that way, it seems far simpler to create a new "Open in Firefox" menu item thats selector checks we are not within a tab type browser (and add the opposite check for the existing items)

That still means duplicating the tracking of tab type between browser.js and Java, https://bugzilla.mozilla.org/show_bug.cgi?id=1351739 does sound like a nice solution to this, I will give that a shot, any guidance would be great

Comment 18

[Jan Henning [:JanH]]

I think that bug 1351834 is the more immediately relevant - without the correct tab type set, this differentiation won't work in the first place for web apps.

(In reply to Dale Harvey (:daleharvey) from comment #17)
> I took a quick look at this and instead of tracking it that way, it seems
> far simpler to create a new "Open in Firefox" menu item thats selector
> checks we are not within a tab type browser (and add the opposite check for
> the existing items)

If that works as well, then great.

> That still means duplicating the tracking of tab type between browser.js and
> Java, https://bugzilla.mozilla.org/show_bug.cgi?id=1351739 does sound like a
> nice solution to this

To be honest I'm not quite sure how that'll help - the session store e.g. still needs to know the tab type, so having it directly available in Gecko as well is easier than always having to query it from Java. Bug 1348686 will make sure that the if the tab type changes, it'll always be updated in both locations.

I'm also already looking at a first implementation for bug 1351739 - depending on how many complications turn up in practice I might have something presentable after the weekend.

Comment 19

[Dale Harvey (:daleharvey)]

oh sorry I wrote that very awkwardly, I meant instead of trying to juggle the tab type changes between gecko and java that 1351739 seems like a much nicer solution, so I will just hold on on this until we see if 1351739 works out, that sound ok?

Comment 21

[Dale Harvey (:daleharvey)]

So thanks to JanH's work this all works together really well, when long press we will have the option to open in new/private tab, when those options are selected we get a little actionbar notification and on pressing that can switch to the relevant tab

I think we may want to as initially proposed change the string from "open in new tab" to "open in Firefox tab", will keep this open for that but reprioritise and remove as blocking.