Closed
Bug 1353857
Opened 8 years ago
Closed 7 years ago
Fix handling of entering/leaving Web App
Categories
(Firefox for Android Graveyard :: Web Apps (PWAs), enhancement, P2)
Tracking
(firefox55 verified)
VERIFIED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | verified |
People
(Reporter: JanH, Assigned: JanH)
References
Details
Attachments
(1 file)
At the moment, the message passed by Gecko (Website:AppEntered/Left) doesn't even include the tab ID, which means that the address bar can be shown when it shouldn't be or vice versa.
To be done after bug 1352997, though, so we can do it properly...
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jh+bugzilla
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8858824 -
Flags: review?(dale)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8858824 [details]
Bug 1353857 - Include the tab ID when notifying about leaving/entering a web app's scope.
https://reviewboard.mozilla.org/r/130846/#review135714
Minor nit, otherwise looks great, cheers
::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java:113
(Diff revision 2)
> }
>
> @Override
> public void handleMessage(final String event, final GeckoBundle message,
> final EventCallback callback) {
> + if (message.containsKey("tabId") && message.getInt("tabId") == mLastSelectedTabId) {
I am not 100% clear on our or Java's coding style preferences, but early returns always seem cleaner for this
if (!message.containsKey("tabId") || message.getInt("tabid") != mLastSelectedTabId) {
return;
}
Feel free to ignore especially if it goes against our standard
Attachment #8858824 -
Flags: review?(dale) → review+
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8858824 [details]
Bug 1353857 - Include the tab ID when notifying about leaving/entering a web app's scope.
https://reviewboard.mozilla.org/r/130846/#review135718
Attachment #8858824 -
Flags: review?(walkingice0204) → review+
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8858824 [details]
Bug 1353857 - Include the tab ID when notifying about leaving/entering a web app's scope.
https://reviewboard.mozilla.org/r/130846/#review135714
> I am not 100% clear on our or Java's coding style preferences, but early returns always seem cleaner for this
>
> if (!message.containsKey("tabId") || message.getInt("tabid") != mLastSelectedTabId) {
> return;
> }
>
> Feel free to ignore especially if it goes against our standard
As things currently stand, you're right. I was just thinking that eventually we might need to handle other events with possibly different pre-conditions which might not be compatible with early returns, but I suppose we can worry about that situation if we actually run into it.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/9ce0883ffd62
Include the tab ID when notifying about leaving/entering a web app's scope. r=daleharvey,walkingice
Backed out because the dependent bug failed to land: https://hg.mozilla.org/integration/autoland/rev/483340ef5362
Flags: needinfo?(jh+bugzilla)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jh+bugzilla)
Comment 12•7 years ago
|
||
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/1219abce0035
Include the tab ID when notifying about leaving/entering a web app's scope. r=daleharvey,walkingice
Comment 13•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 14•7 years ago
|
||
Build: Nightly 2017-05-04
Devices: HTC 10 (Android 6) & Prestigio Grace X5 (Android 4.4.2)
On Twitter & Pokedex.org web apps, when you open an external link, it's opened in a new browser tab, not inside the web app.
Flags: needinfo?(jh+bugzilla)
Comment 15•7 years ago
|
||
Twitter for me opens the link in a new browser tab, which I believe is a change on twitter side so it now behaves the same as chrome
Pokedex behaves the same for me and navigates with the header
Assignee | ||
Comment 16•7 years ago
|
||
- That's not caused by this bug, so if we wanted to change this, this belongs in a separate bug.
- Yup, those links are opening and selecting a new (normal) tab, so because of bug 1351739 this'll trigger an activity switch.
- If we don't want this, this (https://dxr.mozilla.org/mozilla-central/rev/4a6a71f4aa22e4dc3961884ce505ce34bdd799a2/mobile/android/chrome/content/browser.js#3404) is the place to control this behaviour (whether to open a new tab or not), but that's a matter of taste (and if Chrome does it as well, that's one less reason to change our behaviour)
- Re testing with Pokedex, the link at the bottom of the page stays within the tab, but the "About" link in the sidebar menu opens a new tab.
Flags: needinfo?(jh+bugzilla)
Comment 17•7 years ago
|
||
Ah thats how it works, yeh I have been meaning to look into why twitter links were previously opened in place but hadnt got to it. So basically 1351739 fixed _blank links, which is perfect :)
Comment 18•7 years ago
|
||
Yup, you're right, it behaves the same with Chrome. I'll mark this as fixed, thanks!
Status: RESOLVED → VERIFIED
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•