Closed Bug 1632008 Opened 5 years ago Closed 2 years ago

Page.navigate should return loaderId for about:*, file: and data: urls

Categories

(Remote Protocol :: CDP, enhancement, P3)

enhancement
Points:
2

Tracking

(firefox102 fixed)

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: impossibus, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

(Whiteboard: [bidi-m3-mvp])

Attachments

(1 file)

Example

 Page.navigate({"url":"about:blank"})
 Page.navigate({"url":"about:blank"}) = {"frameId":"ECDF1F217C0F19BA543330D28204F18B","loaderId":"E5ECB29B91FA2DF71CCF5C44E9637147"}
 Page.frameStartedLoading({"frameId":"ECDF1F217C0F19BA543330D28204F18B"})
      Page.frameNavigated({"frame":{"id":"ECDF1F217C0F19BA543330D28204F18B","loaderId":"E5ECB29B91FA2DF71CCF5C44E9637147","mimeType":"text/html","securityOrigin":"://","url":"about:blank"}})
      Page.loadEventFired({"timestamp":82917.873615})
 Page.frameStoppedLoading({"frameId":"ECDF1F217C0F19BA543330D28204F18B"})
Page.domContentEventFired({"timestamp":82917.874269
Priority: -- → P3
Whiteboard: [puppeteer-beta2-mvp]
Component: CDP: Page → CDP

(In reply to Julian Descottes [:jdescottes] from bug 1770135 comment #0)

When using Page.navigate we don't return a proper loaderId when the target url does not start with http/https: https://searchfox.org/mozilla-central/rev/7f729f601c0b738f870ae0ed49098f9268e250f9/remote/cdp/domains/parent/Page.jsm#107-110

Because of this, the current implementation of page.goto in puppeteer actually dismisses the event. Navigation works by accident because we incorrectly emit navigatedWithinDocument which tricks puppeteer into thinking that the navigation was a "same document navigation".

However we want to stop emitting those incorrect navigatedWithinDocument events and therefore we should fix navigate to always return a proper loaderId.

Note that this will be irrelevant for puppeteer once https://github.com/puppeteer/puppeteer/pull/8369 (re)lands, because loaderId will no longer be checked, but we need to ensure minimal backward compatibility with the current puppeteer so we can't just ignore the loaderId issue.

Blocks: 1636453

about:* documents will not served via the network layer and as such we won't have a loader id. I wonder if we should just set a random uuid instead, which we would have to store in a map keyed by the browsing context id. Then we could re-use the same id for the various navigation events.

Ah thanks for finding the duplicate, bugzilla didn't suggest it to me when I filed the new one.

Not directly related but I noticed we have a frameIdToLoaderId map already in the content Page domain: https://searchfox.org/mozilla-central/rev/7f729f601c0b738f870ae0ed49098f9268e250f9/remote/cdp/domains/content/Page.jsm#31-36

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

Per CDP documentation, loaderId should be a string so we generate a random uuid here.
To make sure the loaderId will be consistent with other events, we notify the content Page domain
about this loaderId via "_updateLoaderId" which is already used to map browsing context ids to
loader ids.

Summary: Page.navigate should return loaderId for about:* urls → Page.navigate should return loaderId for about:*, file: and data: urls
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d9a06f126bb3 [cdp] Return a randomly generated loaderId for file/data/about navigations r=webdriver-reviewers,whimboo
Points: --- → 2
Whiteboard: [puppeteer-beta2-mvp] → [bidi-m3-mvp]
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: