Browse Source

fix: prevent title change for within page navigation (#46034)

* fix: prevent title change for on page navigation

Co-authored-by: Michaela Laurencin <[email protected]>

* add back and forward testing

Co-authored-by: Michaela Laurencin <[email protected]>

* update Chromium comment

Co-authored-by: Michaela Laurencin <[email protected]>

* remove errant script tag

Co-authored-by: Michaela Laurencin <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Michaela Laurencin <[email protected]>
trop[bot] 1 month ago
parent
commit
99e589c2cf

+ 11 - 9
shell/browser/api/electron_api_web_contents.cc

@@ -2181,6 +2181,17 @@ void WebContents::DidFinishNavigation(
       if (is_main_frame) {
         Emit("did-navigate", url, http_response_code, http_status_text);
       }
+
+      content::NavigationEntry* entry = navigation_handle->GetNavigationEntry();
+
+      // This check is needed due to an issue in Chromium
+      // Upstream is open to patching:
+      // https://bugs.chromium.org/p/chromium/issues/detail?id=1178663
+      // If a history entry has been made and the forward/back call has been
+      // made, proceed with setting the new title
+      if (entry &&
+          (entry->GetTransitionType() & ui::PAGE_TRANSITION_FORWARD_BACK))
+        WebContents::TitleWasSet(entry);
     }
     if (is_guest())
       Emit("load-commit", url, is_main_frame);
@@ -2201,15 +2212,6 @@ void WebContents::DidFinishNavigation(
            frame_process_id, frame_routing_id);
     }
   }
-  content::NavigationEntry* entry = navigation_handle->GetNavigationEntry();
-
-  // This check is needed due to an issue in Chromium
-  // Check the Chromium issue to keep updated:
-  // https://bugs.chromium.org/p/chromium/issues/detail?id=1178663
-  // If a history entry has been made and the forward/back call has been made,
-  // proceed with setting the new title
-  if (entry && (entry->GetTransitionType() & ui::PAGE_TRANSITION_FORWARD_BACK))
-    WebContents::TitleWasSet(entry);
 }
 
 void WebContents::TitleWasSet(content::NavigationEntry* entry) {

+ 21 - 0
spec/api-web-contents-spec.ts

@@ -631,6 +631,17 @@ describe('webContents module', () => {
         w.webContents.navigationHistory.goBack();
         expect(w.webContents.navigationHistory.getActiveIndex()).to.equal(0);
       });
+
+      it('should have the same window title if navigating back within the page', async () => {
+        const title = 'Test';
+        w.webContents.on('did-finish-load', () => {
+          w.setTitle(title);
+          w.loadURL(`file://${fixturesPath}/pages/navigation-history-anchor-in-page.html#next`);
+        });
+        await w.loadURL(`file://${fixturesPath}/pages/navigation-history-anchor-in-page.html`);
+        w.webContents.navigationHistory.goBack();
+        expect(w.getTitle()).to.equal(title);
+      });
     });
 
     describe('navigationHistory.canGoForward and navigationHistory.goForward API', () => {
@@ -653,6 +664,16 @@ describe('webContents module', () => {
         w.webContents.navigationHistory.goForward();
         expect(w.webContents.navigationHistory.getActiveIndex()).to.equal(1);
       });
+
+      it('should have the same window title if navigating forward within the page', async () => {
+        const title = 'Test';
+        w.webContents.on('did-finish-load', () => {
+          w.setTitle(title);
+          w.loadURL(`file://${fixturesPath}/pages/navigation-history-anchor-in-page.html#next`);
+        });
+        await w.loadURL(`file://${fixturesPath}/pages/navigation-history-anchor-in-page.html`);
+        expect(w.getTitle()).to.equal(title);
+      });
     });
 
     describe('navigationHistory.canGoToOffset(index) and navigationHistory.goToOffset(index) API', () => {

+ 5 - 0
spec/fixtures/pages/navigation-history-anchor-in-page.html

@@ -0,0 +1,5 @@
+<html>
+<body>
+  <span id="next">This is content.</span>
+</body>
+</html>