1337264 - URL not properly updated on Reddit when navigating to the next page

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[arv]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160606113944

Steps to reproduce:

1. Navigate to https://m.reddit.com/
2. Scroll to bottom; tap "NEXT" button at bottom right to navigate to the next page of content
3. Tap location bar to notice the URL has updated to reflect the new page (eg, https://www.reddit.com/?count=25&page=1&after=t3_5sf6ns)
4. Kill Firefox from Android task manager
5. Restart Firefox 


Actual results:

* After navigating to the second page, the location bar display is not updated properly, showing only https://www.reddit.com/ until I tap it, when it displays the actual URL of the current page
* After Firefox is restarted, the front page that I navigated away from is restored


Expected results:

* The location should have updated to display the correct URL without having to tap to select the location
* The page I was on when Firefox was killed should have been restored

I suspect the problem is related to a javascript page transition on the Reddit site, but I have not investigated this, so I could be wrong.

Comment 1

[Jan Henning [:JanH]]

(In reply to arv from comment #0)
> * The page I was on when Firefox was killed should have been restored

The history change listener we have on each tab object (https://dxr.mozilla.org/mozilla-central/rev/af8a2573d0f1e9cc6f2ba0ab67d7a702a197f177/mobile/android/chrome/content/browser.js#4511) needs to be hooked up to the session store so as to match desktop behaviour (compare https://dxr.mozilla.org/mozilla-central/rev/af8a2573d0f1e9cc6f2ba0ab67d7a702a197f177/browser/components/sessionstore/content/content-sessionStore.js#323). At the moment we're only listening for DOMTitleChanged, which doesn't trigger here.

> * After navigating to the second page, the location bar display is not
> updated properly, showing only https://www.reddit.com/ until I tap it, when
> it displays the actual URL of the current page

This probably has a similar cause, but needs some further investigation as to which bit actually needs fixing.

Comment 2

[Carmen Fat [:cfat] - Ecosystem QA]

Hello,

Since this issue is related to the URL displaying, I'm moving this report to Awesomescreen component. Please move it to the correct component if I'm wrong. Thank you.

Comment 3

[arv]

Only part of the issue is related to the URL displaying -- the incorrect URL is also being stored as part of the session, and restores to a different page than was being displayed on shutdown.  Should I file the session behaviour as a separate bug?

Comment 4

[Jan Henning [:JanH]]

Attachment: Bug 1337264 - Don't depend on page title changes for updating the displayed URL.

We've been displaying the URL in place of the page title in the toolbar for quite some time now, but still had the old logic in place whereby only title changes would trigger an update of the displayed text. Most of the time this works fine, because
- page navigation usually goes hand in hand with a DOMTitleChanged event, and
- when our loading progress bar stops, we update the displayed text anyway

however a page doing its navigation in-place using some fancy JS logic and the corresponding history APIs etc. can bypass both of these provisions, since it might trigger neither a title change nor a full browser-side page load.

Review commit: https://reviewboard.mozilla.org/r/111798/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111798/

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8836374 [details]
Bug 1337264 - Don't depend on page title changes for updating the displayed URL.

https://reviewboard.mozilla.org/r/111798/#review113268

LGTM. Thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94a2ac4a5806
Don't depend on page title changes for updating the displayed URL. r=sebastian

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/94a2ac4a5806

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Do you think it's worth uplifting this?

Comment 9

[Jan Henning [:JanH]]

Comment on attachment 8836374 [details]
Bug 1337264 - Don't depend on page title changes for updating the displayed URL.

Approval Request Comment
[Feature/Bug causing the regression]: Awesomebar
[User impact if declined]: Displayed URL may not update when a pages uses pushState() for its navigation and doesn't change the page title.
[Is this code covered by automated tests?]: Probably not.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]:  No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This just makes sure that a pure top level location change (without a title change or a full page load) updates the displayed URL as well.
[String changes made/needed]: none

Comment 10

[Gerry Chang [:gchang]]

Hi Brindusa, 
Could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 11

[Brindusa Tot, Desktop QA]

Carmen, could you please verify this bug on latest Nightly?

Comment 12

[Carmen Fat [:cfat] - Ecosystem QA]

Hello, 

Tested on latest Nightly build 54.0a1 (2017-02-15) using the following devices:
- Oneplus Two (Android 6.0.1);
- Samsung Galaxy Note 4 (Android 5.0.1);
- Huawei MediaPad M2 (Android 5.1.1).

Following the STR from the Description results into displaying the updated URL when going to the next page of the website.
I'm marking this as Verified for Firefox 54.

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8836374 [details]
Bug 1337264 - Don't depend on page title changes for updating the displayed URL.

Fix an awesomebar issue on Reddit and was verified on nightly. Aurora53+.

Comment 14

[Julien Cristau [:jcristau]]

(In reply to Jan Henning [:JanH] from comment #9)
> [Is this code covered by automated tests?]: Probably not.

Is there any chance this could be addressed? :)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7f305a9951d3

Comment 16

[Carmen Fat [:cfat] - Ecosystem QA]

Tested on latest Aurora build 53.0a2 (2017-02-17) using the following devices:
- HTC Desire 820  (Android 6.0.1);
- Samsung Galaxy Note 4 (Android 5.0.1);
- Asus ZenPad 8 (Android 6.0.1).

Following the STR from the Description results into displaying the updated URL when going to the next page of the website.
I'm marking this as Verified for Firefox 53.

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 8836374 [details]
Bug 1337264 - Don't depend on page title changes for updating the displayed URL.

update url displayed by fennec even if page title is unchanged; beta52+

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ddf62df9e2e2

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/ddf62df9e2e2