1821579 - Sometimes the page title does not change from the URL.

Status: REOPENED

(Firefox for Android :: Tabs, defect, P3)

Description

[tesmite]

Attachment: Screenshot_20230310-133004_Firefox Nightly.jpg

User Agent: Mozilla/5.0 (Android 9; Mobile; rv:109.0) Gecko/112.0 Firefox/112.0

Steps to reproduce:

1. Open a website (especially likely to occur with Google search).
2. Open a link in the page.
3. Go back to the previous page.

Actual results:

Page names in tab lists, etc. are left as URLs (e.g., "https://www.google.com/*") instead of loading the actual page titles.

Expected results:

The page title (e.g., "** - Google Search") is displayed.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[jackyzy823]

This bug is the same as

1. [Bug]: page title in tabs tray gets replaced by url when forward/back navigating https://github.com/mozilla-mobile/fenix/issues/22667
2. Tab title missing after history back https://github.com/mozilla-mobile/fenix/issues/25659 aka https://bugzilla.mozilla.org/show_bug.cgi?id=1812856

Here's some my observation:

1. It may not be triggered on every site. (It is more likely to be triggered in simple site with less javascript) . for example https://lwn.net -> it's articles -> Go Back could almost 100% be triggered.

2. The main different i found in log is that a) for title displayed , UpdateUrlAction and then UpdateTitleAction triggerd b) however for url displayed , UpdateUrlAction triggered (so ContentState's title is set to "" , then toDisplayTitle set display title to url ) but UpdateTitleAction not triggerd.

   UpdateUrlAction and UpdateTitleAction are in android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt

3. a) for url displayed , the progress bar over address bar went away very fast. b) for title displayed , the progress bar over address bar went away much slower . based on this phenomenon, i guess there're some events shortcut before onTitleChange sent from GeckoView

Besides I think whether title is generated by javascript or in html has no relatation with this bug.
(https://github.com/mozilla-mobile/fenix/issues/20546 aka https://bugzilla.mozilla.org/show_bug.cgi?id=1812833)

I'm not familiar with GeckoView's C++ part, so i can't dig deeper.

Hope information above would help.

Comment 3

[jackyzy823]

After another digging , I found the different between normal and abnormal behaviors.

Normal

GeckoViewProgress: ProgressTracker updateProgress data={"prev":0,"uri":"xxxnormalxxx","locationChange":false,"pageStart":true,"pageStop":false,"firstPaint":false,"pageShow":false,"parsed":false} progress=15
GeckoViewProgress: ProgressTracker onStateChange: isTopLevel=true, flags=0xf0001, status=NS_OK
GeckoViewProgress: ProgressTracker updateProgress data={"prev":15,"uri":"xxxnormalxxx","locationChange":true,"pageStart":true,"pageStop":false,"firstPaint":false,"pageShow":false,"parsed":true} progress=55
GeckoViewProgress: ProgressTracker updateProgress data={"prev":55,"uri":"xxxnormalxxx","locationChange":true,"pageStart":true,"pageStop":false,"firstPaint":true,"pageShow":false,"parsed":true} progress=80
GeckoViewProgress: ProgressTracker updateProgress data={"prev":80,"uri":"xxxnormalxxx","locationChange":true,"pageStart":true,"pageStop":false,"firstPaint":true,"pageShow":true,"parsed":true} progress=100

Abnormal

GeckoViewProgress: ProgressTracker updateProgress data={"prev":0,"uri":"xxxabnormalxxx","locationChange":false,"pageStart":true,"pageStop":false,"firstPaint":false,"pageShow":false,"parsed":false} progress=15
GeckoViewProgress: ProgressTracker onStateChange: isTopLevel=true, flags=0x10f0001, status=NS_OK
GeckoViewProgress: ProgressTracker updateProgress data={"prev":15,"uri":"xxxabnormalxxx","locationChange":true,"pageStart":true,"pageStop":false,"firstPaint":false,"pageShow":true,"parsed":false} progress=100

The key point is flag (normal flags=0xf0001 abnormal flags=0x10f0001,)

and 0x1000000 means const unsigned long STATE_RESTORING = 0x01000000; in https://searchfox.org/mozilla-central/source/uriloader/base/nsIWebProgressListener.idl#151

So when the navigation is with STATE_RESTORING , somehow all DOM related event will not be triggered. You can observe it from log : parsed (related to DOMContentLoaded event) : false, firstpaint (related to MozAfterPaint event) :false . So i guess title related event is not triggered too.

So when STATE_RESTORING , GeckoView should send GeckoView:PageTitleChanged in https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewProgress.sys.mjs#252 too (not only when pagetitlechanged in https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewContent.sys.mjs#223)

If STATE_RESTORING 's behavior is intended, here's a workable fix for your consideration. (If STATE_RESTORING 's behavior is not intended , then well maybe Gecko core part need to be reviewed)

diff --git a/mobile/android/modules/geckoview/GeckoViewProgress.sys.mjs b/mobile/android/modules/geckoview/GeckoViewProgress.sys.mjs
index 4606a52b99e28..1fd560a09df00 100644
--- a/mobile/android/modules/geckoview/GeckoViewProgress.sys.mjs
+++ b/mobile/android/modules/geckoview/GeckoViewProgress.sys.mjs
@@ -284,6 +284,12 @@ class ProgressTracker extends Tracker {
       probe.start();
       this.start(displaySpec);
     }
+    if ((aStateFlags & Ci.nsIWebProgressListener.STATE_RESTORING) != 0) {
+      this.eventDispatcher.sendRequest({
+        type: "GeckoView:PageTitleChanged",
+        title: this.browser.contentTitle,
+      });
+    }
   }
 
   onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {

Comment 4

[Christian Sadilek [:csadilek]]

The workaround/solution above seems OK, but is best discussed on a patch. The downside is that we're mixing the progress and content delegates here. It's also not clear to me if this.browser.contenTitle is guaranteed to be up-to-date at this point.

Comment 5

[jackyzy823]

Attachment: Bug 1821579 - Send GeckoView:PageTitleChanged event when STATE_RESTORING. r=#geckoview-reviewers,csadilek!

Comment 6

[jackyzy823]

The downside is that we're mixing the progress and content delegates here.

AFAIK, There's no other way/method to get STATE_RESTORING except in progress handler. and since the content is not actually re-loaded when STATE_RESTORING , so i think we shouldn't make this stronglly related to content delegate.

Another solution is make "GeckoView:LocationChange" message contains title (whatever the content with/without STATE_RESTORING ), however it will break API and It's also not clear if this.browser.contenTitle is guaranteed to be up-to-date at this point

The final solution is let Gecko core part send titlechanged event even with STATE_RESTORING , and i don't know how and where to work with.

It's also not clear to me if this.browser.contenTitle is guaranteed to be up-to-date at this point.

Here's some guess:
ContentTitle from this.browsingContext?.currentWindowGlobal?.documentTitle and i guess currentWindowGlobal?.documentTitle is GetDocumentTitle and it will only be changed in RecvUpdateDocumentTitle

So if without STATE_RESTORING , the page will load again , and RecvUpdateDocumentTitle will be called and title is updated and onTitleChange sent.

if with STATE_RESTORING , maybe RecvUpdateDocumentTitle will not be called but mdocumentTitle will not be changed. the title keeps the same as it first time loaded (The first time load RecvUpdateDocumentTitle was definitely called.)

---

And if you feel comfortable to be discussed there ,then there it is the patch.

Comment 7

[Christian Sadilek [:csadilek]]

Thanks for the patch. I think this will need some more discussion on the patch by the team.

Comment 8

[Makoto Kato [:m_kato]]

Attachment: Bug 1821579 - Add geckoview-junit test. r=#geckoview-reviewers

Comment 9

[jackyzy823]

(In reply to Christian Sadilek [:csadilek] from comment #7)

Thanks for the patch. I think this will need some more discussion on the patch by the team.

It would be helpful that you could review and approve that patch as i adding you as a blocking reviewer. A member of geckoview-reviewers have reviewed and approved that patch.

Thanks.

Comment 10

[Christian Sadilek [:csadilek]]

OK, reviewed.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6c7ba4e73ce
Send GeckoView:PageTitleChanged event when STATE_RESTORING. r=csadilek,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/b3dbc42ea7c1
Add geckoview-junit test. r=geckoview-reviewers,ohall

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e6c7ba4e73ce
https://hg.mozilla.org/mozilla-central/rev/b3dbc42ea7c1

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jackyzy823, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox113 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is this something we should uplift to mozilla-release for the next scheduled Fenix dot release? Please nominate for approval if so!

Comment 15

[tesmite]

This bug has not been fixed in Firefox 120 and will be reopened.
Also bug 1766676 has not been fixed and seems to have a similar cause, so why is it closed?

Comment 16

[jackyzy823]

Hello :tesmite , I can't reproduce this issue on Google site with Firefox 120. Could you share a video about that?

Comment 17

[tesmite]

Attachment: screen-20231031~2.mp4

Ah, yes, it has indeed almost stopped happening in Google searches.
However, there are still a number of cases where this problem occurs, such as when accessing other sites from a Google shopping search, so I thought the fundamental cause had not been fixed.

Comment 18

[tesmite]

Attachment: Screenshot_20231031-1246.png

Also, bug 1766676 has not been fixed and will almost certainly cause problems in DuckDuckGo when searching.

Comment 19

[Jeff Boek [:boek]]

Hey Jacky, is this something you're still interested in continuing to investigate?

Comment 20

[jackyzy823]

Sorry that i didn't receive the email notification for :tesmite 's reply.

---

I have debugged webpage on duckduckgo , I can confirm that duckduckgo's issue is related to bug 1843682 ( pushState/replaceState will cause title empty and display url instead)

Related code from https://duckduckgo.com/dist/util/u.1af54b143744018d4972.js

        this._stateTimeout = setTimeout(
          function () {
            var e = this._history.state ||
            {
            };
            e[t] = this.curState,
            this._pushing ? this._history.pushState(e, this, document.location.origin + this._curPath) : this._history.replaceState(e, null, document.location.origin + this._curPath),
            this._callbacks.forEach((function (e) {
              e()
            })),
            this._callbacks = [],
            delete this._pushing
          }.bind(this),

I added break point at this._pushing ? , before this line executed , the title of new search term displayed , after this line executed, the title displays url.

---

I have no ability to visit Rakuten or display item from Rakuten in Google shopping , so i'm not sure if Rakuten's issue is caused by the same bug as duckduckgo.

:tesmite Could you provide another case ? Thanks.

Comment 21

[tesmite]

At the time of my previous post, the problem was also occurring on Amazon.com, but due to some kind of modification, I have not been able to check the problem on that site, and currently I can only see the problem on DuckDuckGo (always occurring) and Rakuten Ichiba (some pages).
I will post any other sites where the problem is confirmed as soon as possible.
If possible, I would appreciate it if you could check the problem by setting the "Region for Search Results" to Japan in the Google search settings, so that the Google shopping search will be Japanese and the search results will include Rakuten Ichiba.

Comment 22

[Tom25519]

This issue also happens on www.zhihu.com, for example, https://www.zhihu.com/question/525350115.

Comment 24

[Kurt Felton]

This issue happens on all ESPN links as well.

Comment 25

[Kurt Felton]

For ESPN.com, Reddit.com, and Trakt.tv links when you refresh the page, the titles will show correctly after a refresh on the page. It doesn't show initially on first click / tap of link.

For ESPN.com to replicate, go to ESPN.com than click a Top Headline link towards middle of page.

Comment 26

[Jonathan Almeida [:jonalmeida]]

Linking the connect user reports (I think this Kurt's as well).