1356563 - Race condition on startup that can break opening of initial/restored tabs

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Jan Henning [:JanH]]

Attachment: Minimal profile for reproducing this issue

To recap briefly:
On a high-level basis, our startup looks like this:
- If we're restoring the last session, the Java session parser opens all tabs contained in the session file. This also queues (since Gecko won't be running yet) up an equivalent number of "Tab:Load" messages, so those tabs are opened in Gecko as well.
- We then send (or more precisely, queue) a "Session:Restore" message to Gecko - if we aren't actually restoring, it's empty, otherwise it contains the session file data with fixed up tab IDs
- Finally,
-- if we aren't restoring, we open our default tab/the user's homepage
-- if we were launched through an intent with an URL, we open a new tab to load that page, which queues up another "Tab:Load" message

Then, when Gecko is starting up, all those events are processed, that is
1.) all restored tabs are opened
2.) the session store restores the session history data and any other state for those tabs and
3.) finally any additional startup tab is opened.

If you enable session store debug logging (about:config - browser.sessionstore.debug_logging) and watch a startup, you can see that first there are a number of onTabbAdd() calls, like:
> 04-14 13:22:13.632 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 0, aNoNotification = undefined
> 04-14 13:22:13.680 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 1, aNoNotification = undefined
> 04-14 13:22:13.728 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 2, aNoNotification = undefined
> 04-14 13:22:13.777 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 3, aNoNotification = undefined
> 04-14 13:22:13.871 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 4, aNoNotification = undefined
> 04-14 13:22:13.956 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 5, aNoNotification = undefined
> 04-14 13:22:14.004 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 6, aNoNotification = undefined
> 04-14 13:22:14.477 24643 24671 D GeckoSessionStore: onTabAdd() ran for tab 7, aNoNotification = undefined

Normally, the session store attaches itself to the window (https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/components/SessionStore.js#263, https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/components/SessionStore.js#548) before step 1.), so all those onTabAdd() calls are happening in response the the queued messages from the Java side being processed (you can see that aNoNotifciation = undefined, which means that those onTabAdd() calls were *not* triggered through onWindowOpen() (https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/components/SessionStore.js#572), which sets that parameter to true).


During the last months however, we've had a number of bug reports about tabs not opening properly on startup - the loading indicator would be stuck at a low percentage, the tab was blank and didn't react to anything and if Firefox was killed and reopened, those unresponsive tabs wouldn't be restored any more.

Very recently, I've started seeing something like that as well. As far as immediate causes go, I've eventually tracked it down to the following combination of factors:
- have the Stylish add-on installed and enabled
- have uBlock Origin 1.12.00 installed (1.11.4 and earlier didn't cause any trouble)
- use a Firefox build after bug 1344892 has landed

If all three factors are taken together, then at least on my phone, startup session restoring reliably fails. In the logs, I don't see any onTabAdd() calls in the session store, and the console is showing this:
> 04-14 13:06:08.859 22196 22217 E GeckoConsole: [JavaScript Error: "SessionStore: TypeError: this._windows[window.__SSID] is undefined" {file: "jar:jar:file:///data/app/org.mozilla.fennec_jan-2/base.apk!/assets/omni.ja!/components/SessionStore.js" line: 1629}]
> 04-14 13:06:08.859 22196 22217 E GeckoConsole: SessionStore.prototype.restoreLastSession<@jar:jar:file:///data/app/org.mozilla.fennec_jan-2/base.apk!/assets/omni.ja!/components/SessionStore.js:1629:7
> 04-14 13:06:08.859 22196 22217 E GeckoConsole: TaskImpl_run@resource://gre/modules/Task.jsm:319:42
> 04-14 13:06:08.859 22196 22217 E GeckoConsole: TaskImpl@resource://gre/modules/Task.jsm:277:3
> 04-14 13:06:08.859 22196 22217 E GeckoConsole: asyncFunction@resource://gre/modules/Task.jsm:252:14
> 04-14 13:06:08.859 22196 22217 E GeckoConsole: ss_onEvent@jar:jar:file:///data/app/org.mozilla.fennec_jan-2/base.apk!/assets/omni.ja!/components/SessionStore.js:200:11
> 04-14 13:06:08.862 22196 22217 E GeckoConsole: [JavaScript Error: "TypeError: BrowserApp.deck is null" {file: "chrome://browser/content/browser.js" line: 3544}]

Attempting to switch between any of the now broken startup tabs further shows
> 04-14 13:18:50.191 22196 22217 E GeckoConsole: [JavaScript Error: "TypeError: aTab is null" {file: "chrome://browser/content/browser.js" line: 1434}]

Any tabs that are subsequently opened seem to function normally and are recored in session store as well, although I cannot rule out any remaining side effects.

I'm not quite sure which change in uBlock exactly triggered this, but out of the changelog, the fix for bug 1352741 (https://github.com/gorhill/uBlock/issues/2493) seems the most suspicious.
In any case, if I take away any single one out of those three factors, then startup proceeds normally...

...most of the time:
I've tracked down bug 1344892 through a mozregression run with the attached profile, which means that Stylish and uBlock 1.12.00 were enabled and only the Firefox build varied.

Occasionally, even a supposedly good build would exhibit strange behaviour, in that when monitoring the onTabAdd() calls, some times the first few tabs were missing. That is instead of logging an onTabAdd() call for each tab ID from 0 through to the end (compare above), the first onTabAdd() call shown would be for some later tab ID, e.g. only starting from tab ID 2. If I then tried interacting with the tabs in the UI, any tabs for which an onTabAdd() call was logged would behave normally, while the first few tabs which were missing in the logs would be broken.

I think (but I'm not absolutely sure) that on some very rare occasions I've seen startup restoring completely fail (that is no onTabAdd() calls and the session store error in the browser console) even on a "good" build but with Stylish and uBlock 1.12.00 enabled.

So while bug 1344892 has definitively exacerbated things, it seems that even without out it we have some sort of race condition between message/event dispatching from Java and Gecko-side startup going on:
- We can fail to properly open tabs on the Gecko side in response to those queued "Tab:Load" messages
-- the missing onTabAdd() calls
-- those tabs are effectively dead, you can't interact in any way with them
-- attempting to switch to such a tab in the UI shows "TypeError: aTab is null", i.e. the tab is missing on the Gecko side

- The "Session:Restore" message can be processed at a point in time where we're not actually ready to deal with it yet
-- The tabs haven't been opened yet (see above)
-- "SessionStore: TypeError: this._windows[window.__SSID] is undefined" means that the session store is attempting to interact with a window for which onWindowAdd() hasn't been called yet (presumably when attempting to restore closed tabs here: https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/components/SessionStore.js#1438)

- The strange error of "TypeError: BrowserApp.deck is null" {file: "chrome://browser/content/browser.js" line: 3544} - why would the deck be null if JS-BrowserApp (browser.js) is already running?

- On the other hand any tabs opened afterwards seem to function normally, so whatever wasn't ready during startup eventually initialises itself.

On top of that, as I already mentioned we've also received a number of bug reports with similar symptoms dating back to both before bug 1344892 and the recent uBlock update.

Comment 1

[Jan Henning [:JanH]]

Attachment: Log of successful startup

So I've added a little bit more of logging to the session store, which helped me noticed the following interesting bit of info:

During a normal startup, everything (tabs opening, session store attaching itself to the window, session history restoring) happens *after*

04-14 15:09:39.595 6132-6155 D/GeckoBrowser: zerdatime 1492175379595 - browser chrome startup finished.

i.e. https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/mobile/android/chrome/content/browser.js#384.

Comment 2

[Jan Henning [:JanH]]

Attachment: Log of failed startup

On a failed startup attempt on the other hand, "browser chrome startup finished" appears only after the session store has already attempted to restore the session, i.e. *after* "Session:Restore" had already been dispatched from Java to Gecko (as evidenced by the fact that the session store is attempting to run the corresponding code).

That explains why the tabs fail to open (browser.js's "Tab:Load" listener is registered only *after* "browser chrome startup finished", so when the messages were dispatched, nobody was there to receive and act on them) and why the session store fails to interact with the browser window (domwindowopened/window load happens at some point near "browser chrome startup finished", so hadn't yet happened/was still in progress when "Session:Restore" was dispatched).

So it would still be interesting to find out in what way the combination of bug 1344892, Stylish and uBlock 1.12.00 is actually affecting our startup timings, but in any case there is a definitive race condition going on that needs fixing.

Comment 6

[Jan Henning [:JanH]]

Corresponding uBlock issue:
https://github.com/gorhill/uBlock/issues/2535

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Only set global ready state on native widget loading (v1)

Our "chrome-document-loaded" observer may detect several different types
of widgets that can exist in the parent process, including the Android
nsWindow, PuppetWidget, etc. We should only set the global state to
ready when the first top-level nsWindow has loaded, and not just any
window.

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Can you see if this patch makes it better? With this patch, the "chrome startup finished" message should always come before the session store messages.

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8858885 [details] [diff] [review]
Only set global ready state on native widget loading (v1)

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

::: widget/android/nsAppShell.cpp
@@ +593,5 @@
> +        // widget is a top-level window and that its NS_NATIVE_WIDGET value is
> +        // non-null, which is not the case for non-native widgets like
> +        // PuppetWidget.
> +        if (widget &&
> +            widget->WindowType() == nsWindowType::eWindowType_toplevel &&

Do we have more than one top-level widget in the parent process? The GetNativeData() thing is kinda ugly, so would be good to avoid it if we can.

Comment 10

[Jan Henning [:JanH]]

Comment on attachment 8858885 [details] [diff] [review]
Only set global ready state on native widget loading (v1)

Yup, seems to be working fine this way.

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #9)
> Comment on attachment 8858885 [details] [diff] [review]
> Only set global ready state on native widget loading (v1)
> 
> Review of attachment 8858885 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/android/nsAppShell.cpp
> @@ +593,5 @@
> > +        // widget is a top-level window and that its NS_NATIVE_WIDGET value is
> > +        // non-null, which is not the case for non-native widgets like
> > +        // PuppetWidget.
> > +        if (widget &&
> > +            widget->WindowType() == nsWindowType::eWindowType_toplevel &&
> 
> Do we have more than one top-level widget in the parent process? The
> GetNativeData() thing is kinda ugly, so would be good to avoid it if we can.

Right now it appears only nsWindow will be top-level, but I'm concerned that we may get other top-level windows in the future that are not nsWindow, e.g. if someone creates a top-level HeadlessWidget.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/676c7c54953c
Only set global ready state on native widget loading; r=snorp

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/676c7c54953c

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8858885 [details] [diff] [review]
Only set global ready state on native widget loading (v1)

Approval Request Comment
[Feature/Bug causing the regression]: Unknown
[User impact if declined]: Failed startup resulting in tab content not appearing
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Open a link from external app (such as gmail). Intermittent.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: A little
[Why is the change risky/not risky?]: Changes critical startup code
[String changes made/needed]: None

I was not aware this bug affected 53. We probably need to hold the Android release for this patch. We may need a rebased patch, which jchen will provide.

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

I am not sure why this was marked tracking-fennec+. In the future, if we have a known very bad regression, it should track a specific release for fixing.

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Patch for uplift

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8858885 [details] [diff] [review]
Only set global ready state on native widget loading (v1)

[Triage Comment]
Taking it on all branches.

could you land this asap?
thanks

Comment 18

[Aki Sasaki (not active)]

Landed on m-r: https://hg.mozilla.org/releases/mozilla-release/rev/4214d2ff34c7
Still needs landing on m-b

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Gerry, heads up, we will need this patch in 54 beta 1 build (which I think you may be planning tonight.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Stefan, kevin, can you help test if we get the 53.0.1 build done this afternoon? We want to release it as soon as possible. 

Looks like there are/were problems with session store (restoring tabs on restart) and with opening external links. 

Marking this as a blocking issue for 53 and 54.

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Snorp, Jim, do we know which bug/fix regressed this? Just wondering how certain we are that this is a new regression in 53.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Stefan, kbrosnan, anyone else who feels like testing: Can you check asap to see if 52.0.2 Fennec is affected in the same way? We don't know that it doesn't.   And, if it is not new in 53.0 then we can move a bit more slowly/carefully here.

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

It looks like things could've potentially been caused by bug 1243069, which was added in Firefox 47, but it sounds like something else has occurred since then to make this bug more likely to occur.

Comment 24

[Jan Henning [:JanH]]

To distill that point from my wall of text in the initial comment:

For me there were three factors that made this almost 100% reproducible on my phone:
- having Stylish (2.0.7, but that's been the current version since last summer) installed and enabled
- having uBlock Origin 1.12.0 or higher (published on 9 April) installed and enabled
- running a Nightly build after bug 1344892 landed

If I use a 55 Nightly from before bug 1344892, I can still see this issue, but not as frequent and sometimes its affecting only the first few tabs (i.e. the chrome window looses the race by a very narrow margin).

On Release, I haven't noticed this so far in the time since the uBlock 1.12.0 update.

If you look at the duplicates of this bug and bug 1324205, though, other people have been seeing this since already a few versions back (maybe different phones with different performance characteristics, or other add-ons that potentially affect our startup timings).

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Thanks Snorp, Jan. Is the presence of these two add-ons a necessity to repro this bug? If that is true, then the overall severity of this issue goes down and is perhaps not release blocking. We would want to fix this in the next dot release but we wudnt rush to push that dot release out the door tomorrow. We prefer to push a well tested dot release that also fixes a few other issues that might show up in the next few days. Wdyt?

Amy Tsay just told me that on Fennec the add-on usage for these add-ons are: Stylish is ~6K, uBlock ~240K

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Stefan wasn't able to reproduce this issue in 52 after some testing this afternoon. I am not sure if that's with or without the particular addons mentioned. Others have been testing a bit without seeing an obvious problem. 

I agree with Ritu that we don't need to rush this out. We can take more time for investigation. 

Since we thought there was a chance that it would be a widespread problem, we already have a 53.0.1 build with the fix, in testing.

Comment 27

[Stefan [:StefanG_QA]]

So far the I can not reproduce the issue on Firefox 52.0.2. Every time the session is restored properly for me and all external links (ex. from gmail) are open correctly with the mentioned add-ons installed on the device. 

I'm working on reproducing the issue, then I will proceed with verifying the fix on build 53.0.1

Comment 28

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I do not have Stylish, but I've noticed this issue occasionally (on Nightly) for at least a few weeks, but rarely enough that I can't reproduce.

The only addon I have in common is uBlock, I also have Switchboard Experiments (generally whatever the latest versions of those were, uBlock 1.12.1 and Switchboard 0.6.0 currently). I have had Activity Stream enabled for as long as I've had this issue, but I haven't tried disabling any of these.

Comment 29

[Stefan [:StefanG_QA]]

I have no luck with reproducing the issue on 53.0 nor on 53.0.1. Every time the session is restored correctly and I`m able to navigate and load each tab from the previous session. Also all pages from external links are loaded properly(I have noticed that browser needs a couple of seconds to load the pages like cnn.com, usatoday.com or etc.).

I have installed: 
-Stylish 2.0.7
-uBlock Origin 1.12.1 (updated Apr 16, 2017)

Comment 30

[Daniel Veditz [:dveditz]]

I have had Stylish and uBlock Origin for a long time and use Fennec Beta (so I've run that combination with Fx53 for 6 weeks or so). I lost my tabs once a week or two ago. I have also noticed a couple of times when sites weren't loading and I had to force quit Fennec and restart to get it working again (but I didn't lose my tabs). Doesn't seem to warrant an emergency rushed or risky fix.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/584df356a66b

Comment 32

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Ritu Kothari (:ritu) from comment #25)
> Thanks Snorp, Jan. Is the presence of these two add-ons a necessity to repro
> this bug? If that is true, then the overall severity of this issue goes down
> and is perhaps not release blocking. We would want to fix this in the next
> dot release but we wudnt rush to push that dot release out the door
> tomorrow. We prefer to push a well tested dot release that also fixes a few
> other issues that might show up in the next few days. Wdyt?

In theory any add-on could cause us to create a hidden window that triggers this bug on occasion, but the corollary to that is you do need at least one add-on installed (in addition to the bundled ones). That, along with the fact that it seems to have been exacerbated only recently by bug 1344892, seems to indicate that it wouldn't be a deal-breaker if we wait a few days.

Comment 33

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Retracting my comment 28, I think what I was seeing (rarely) without Stylish was a different issue. Sorry for the clutter.

Comment 34

[Kevin Brosnan [Ex-Mozilla]]

I would recommend that anyone trying to reproduce this enable the developer options in Android and set don't keep activities along with the extensions mentioned in this bug. I've not had a lot of time to look at this yet.

Comment 35

[Oana Horvath [:ohorvath]]

I was able to reproduce the bug, following the steps mentioned in comment 24:
"- having Stylish (2.0.7, but that's been the current version since last summer) installed and enabled
- having uBlock Origin 1.12.0 or higher (published on 9 April) installed and enabled
- running a Nightly build after bug 1344892 landed" -- I had the 2017-16-04 Nightly build installed. 
I could also reproduce the bug on RC 53.0.1.

In addition to these steps:
I've opened more than 10 tabs from an external app (Google news), with the tab queue off. At some point, switching trough tabs, the pages are not loading anymore, the tabs are blank white. That's when I've killed the browser task and reopened it. The tabs are gone.

Comment 36

[Oana Horvath [:ohorvath]]

Reproduced on an LG G4 (Android 5.1).

Comment 37

[James Willcox (:snorp) ([e-mail]) (he/him)]

As Jim said, I think this can wait for a dot release.

Comment 38

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Ioana, in your sign off email you mentioned that this issue is reproducible on 53.0.1. Does that mean this fix is not effective?  Or was that a typo in the email? Please clarify.

Comment 39

[Ioana Chiorean]

(In reply to Oana Horvath from comment #35)
> I was able to reproduce the bug, following the steps mentioned in comment 24:
> "- having Stylish (2.0.7, but that's been the current version since last
> summer) installed and enabled
> - having uBlock Origin 1.12.0 or higher (published on 9 April) installed and
> enabled
> - running a Nightly build after bug 1344892 landed" -- I had the 2017-16-04
> Nightly build installed. 
> I could also reproduce the bug on RC 53.0.1.

Hey Ritu, 
There was no typo. I also set the status to yellow and advice not to ship it. We did reproduce it - as Oana me ruoned in The above comment. Out of 3 devices we were able to reproduce it on one. We need to further investigate it and try several other devices.

Comment 40

[Jan Henning [:JanH]]

I'm checking the 53 and 53.0.1 builds myself to see whether I can spot anything on my phone, but could you please turn on "consoleservice.logcat" and "browser.sessionstore.debug_logging" in about:config and provide a logcat for a startup where you think you're seeing this?

Comment 41

[Ioana Chiorean]

We can look at that, yes,but in a few hours as the devices are in the office. I will NI to Oana as she was a letter to reproduce it.

Comment 42

[Jan Henning [:JanH]]

Actually I could reproduce the original issue on 53 as well.

On 53.0.1 on the other hand, it looks like none of the messages (open tabs, restore session, ...) that are queued on startup are ever sent on Gecko - I don't see any JS errors in the log, but no tabs opening either. The Quit button in the main menu is not functional, either. Opening new tabs afterwards on the other hand seems to use a slightly different messaging code path between the UI and Gecko and still works.

Comment 43

[Jan Henning [:JanH]]

And now of course all of a sudden I can no longer reproduce it...

Comment 44

[Jan Henning [:JanH]]

Attachment: 53.0.1 log 1.txt

I've downgraded to 52.0.2 from a backup and didn't see anything strange there.

After updating to 53, I could definitively reproduce this issue.

After updating to 53.0.1, I once again ran into the strange behaviour mentioned in comment 53.

That is on the first startup, the UI restored the tabs from the session store file in the UI, but
- once again no tabs were opened in Gecko
- this time however it seems like the session store didn't successfully receive "Session:Restore" (https://dxr.mozilla.org/mozilla-release/source/mobile/android/components/SessionStore.js#289) either, because no data is saved and if I attach the JS debugger, it turns out the this._startupRestoreFinished is still false.
- Quitting from the menu (https://dxr.mozilla.org/mozilla-release/rev/f50b8b33a3523b542db42008c27551edf1baad58/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#603) doesn't work.

I've noticed that on 53, both "Session:Restore" and "Browser:Quit" still go through the observer service, so I wonder whether that could somehow interfere with this patch?

Comment 45

[Jan Henning [:JanH]]

(In reply to Jan Henning [:JanH] from comment #44)
> After updating to 53.0.1, I once again ran into the strange behaviour
> mentioned in comment 53.

Argh, that should be comment 42.

Comment 46

[Jan Henning [:JanH]]

So it seems like Tab:Load already uses the Event Dispatcher, but somehow on 53.0.1 the race condition can probably still exist (meaning the queued Tab:Load messages are sent before browser.js had a chance to register its corresponding listener) and on top everything going through the observer service is severely broken, although only in combination with the above race - if I manage to see a normal startup (e.g. by deactivating Stylish/uBlock), then Session:Restore/quitting/the back button [1] works as well.

[1] In the broken case, both short and long pressing of the back button doesn't have any effect.

Comment 47

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Fix-up for uplifted patch

Sorry, I botched the uplift patch that made it not work under the original test case. This patch, on top of the already uplifted patch, should fix the original bug.

Approval Request Comment
[Feature/Bug causing the regression]: This bug
[User impact if declined]: Fennec doesn't work under the original test case
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Verified locally
[Needs manual test from QE? If yes, steps to reproduce]: Yes, see above
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simple two-line fix to correct the botched uplift patch
[String changes made/needed]: None

Comment 49

[Jan Henning [:JanH]]

Marking 54 as affected again until fix-up is in place.

Comment 50

[Gerry Chang [:gchang]]

Comment on attachment 8860162 [details] [diff] [review]
Fix-up for uplifted patch

Beta54+. Should be in 54 beta 2.

Comment 51

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8860162 [details] [diff] [review]
Fix-up for uplifted patch

Let's get this in place for a Fennec dot release sometime in the next couple of weeks (timing still uncertain).

Comment 52

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8dbf9446a944

Comment 53

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/3dcabe8fddee

Comment 54

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
- LG G4 (Android 5.1.1);
- Nexus 6 (Android 7.0).

Hello,

I've verified this issue in the 54.0b2 Beta build and using the instructions from Comment 24 and Comment 35 could not reproduce it. It would seem that the issue is fixed.

Comment 55

[nucrap]

So in which mobile Firefox release will this be fixed? Sorry but I do not understand those cryptic statuses and tracking flags :/

Comment 56

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to nucrap from comment #55)
> So in which mobile Firefox release will this be fixed? Sorry but I do not
> understand those cryptic statuses and tracking flags :/

The fix has landed for an upcoming dot release of Firefox 53. Date yet to be determined. It should be in 52b2 as well, which should be out sometime later today.

Comment 57

[Jan Henning [:JanH]]

(In reply to Wes Kocher (:KWierso) from comment #56)
> The fix has landed for an upcoming dot release of Firefox 53. Date yet to be
> determined. It should be in 52b2 as well

54b2 :-)

Comment 58

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
- LG G4 (Android 5.1.1);
- Honor 8 (Android 6.0).
Build: 
 - 53.0.1 (Build 2)

Comment 59

[Ioana Chiorean]

Per Comment 54 and Comment 58