Closed Bug 1245516 Opened 4 years ago Closed 4 years ago

launch from homescreen bookmark always opens new tab and sometimes does nothing

Categories

(Firefox for Android :: General, defect)

44 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 47
Tracking Status
firefox44 + verified
firefox45 + verified
firefox46 + verified
firefox47 --- verified
fennec 44+ ---

People

(Reporter: bkelly, Assigned: ahunt)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

On my android M nexus 5x I try to use web pages bookmarked to the homescreen as much as possible.  One I use heavily is twitter.

In the past twitter would open in the same tab unless I had navigated the previous twitter tab to a different URL.

After upgrading to 44 I now seem to get a new tab every time I click the homescreen bookmark icon regardless of previous tab URLs.

Also, I have noticed that if fennec is left open long enough, sometimes click my twitter bookmark does nothing.  It opens fennec to its last screen, but does not open a new tab.  I have to kill fennec and restart it to get out of that state.
tracking-fennec: --- → ?
This changeset looks suspicious:
http://hg.mozilla.org/mozilla-central/rev/5a6b63cd300e
Flags: needinfo?(ahunt)
Uggh, I shadowed originHost there - this should hopefully be a very quick fix.
Assignee: nobody → ahunt
Flags: needinfo?(ahunt)
Sounds like this needs to be uplifted to Fx44?
My patch fixes part 1, i.e. we can now open homescreen shortcuts in the existing tab if available.

We still have some issues, which I suspect are unrelated to my previous changes:

1. Ben reported that we don't always open a tab if needed (I have no idea what kind of state would cause this)
2. We unncessarily open a new tab if Fennec isn't running when the shortcut is selected (either kill the app manually, or use "Don't keep activities").

(Note to self: we should look into writing tests for these issues too.)
[Tracking Requested - why for this release]: We shipped a regression in homescreen shortcuts in 44. I'm not sure that this merits a dot release, but it would be a good ridealong if we do happen to spin another release (e.g. for the Adjust problem).
Comment on attachment 8715543 [details]
MozReview Request: Bug 1245516 - Don't shadow originHost, so we can process appOrigin correctly r=margaret

https://reviewboard.mozilla.org/r/33503/#review30295

Sigh... I'm embarassed I missed this in my original review.
Attachment #8715543 - Flags: review?(margaret.leibovic) → review+
tracking-fennec: ? → 44+
Comment on attachment 8715543 [details]
MozReview Request: Bug 1245516 - Don't shadow originHost, so we can process appOrigin correctly r=margaret

Approval Request Comment
[Feature/regressing bug #]: Regression caused in Bug 1208525 
[User impact if declined]: Homescreen shortcuts always open new tab, instead of reusing the existing tab if available.
[Describe test coverage new/current, TreeHerder]: Manual testing.
[Risks and why]: Low risk - variable is no longer shadowed, meaning the correct value is used again.
[String/UUID change made/needed]: none.
Attachment #8715543 - Flags: approval-mozilla-release?
Attachment #8715543 - Flags: approval-mozilla-beta?
Attachment #8715543 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/integration/fx-team/rev/21ff9b18942e1ef3572de8eed08efd2ee92202b2
Bug 1245516 - Don't shadow originHost, so we can process appOrigin correctly r=margaret
I will most likely take this fix (if it is verified in time) before I gtb Fennec 44.0.1 tomorrow or Friday.
https://hg.mozilla.org/mozilla-central/rev/21ff9b18942e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment on attachment 8715543 [details]
MozReview Request: Bug 1245516 - Don't shadow originHost, so we can process appOrigin correctly r=margaret

Fix a regression, taking it.
Attachment #8715543 - Flags: approval-mozilla-beta?
Attachment #8715543 - Flags: approval-mozilla-beta+
Attachment #8715543 - Flags: approval-mozilla-aurora?
Attachment #8715543 - Flags: approval-mozilla-aurora+
Should be in 45 beta 4.
Comment on attachment 8715543 [details]
MozReview Request: Bug 1245516 - Don't shadow originHost, so we can process appOrigin correctly r=margaret

Let's take it to do a potential build 3
Attachment #8715543 - Flags: approval-mozilla-release? → approval-mozilla-release+
Is it possible to write an automated test that would catch this regression in the future?
Flags: needinfo?(ahunt)
Flags: in-testsuite?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)
> Is it possible to write an automated test that would catch this regression
> in the future?

Yes, it should be possible.
Verified as fixed on Nexus 9, on Firefox 44.0.2 build 4
Verified as fixed on Nexus 9, on Firefox 44.0.2 build 4
Verified as fixed on Nexus 9, on Firefox 45 Beta 6
Verified as fixed using:
Device: One A2001 (Android 5.1.1) 
Build: Firefox for Android 46.0a2 (2016-02-16)
Verified as fixed using:
Device: One A2001 (Android 5.1.1) 
Build: Firefox for Android 47.0a1 (2016-02-18)
Status: RESOLVED → VERIFIED
Blocks: 1249318
(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)
> Is it possible to write an automated test that would catch this regression
> in the future?

I've filed a followup bug to add the tests, and also fix this for all cases (since we still open a new tab if Firefox isn't running), see Bug 1245516.
Flags: needinfo?(ahunt)
Version: unspecified → 44 Branch
You need to log in before you can comment on or make changes to this bug.