Closed Bug 1610452 Opened 5 years ago Closed 5 years ago

Page.navigate doesn't throw error for invalid URLs

Categories

(Remote Protocol :: CDP, task, P3)

task

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1599257

People

(Reporter: whimboo, Unassigned, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lang=js])

User Story

To get familiar with the Remote Protocol, and how to submit patches and request review on those, please read: https://firefox-source-docs.mozilla.org/remote/index.html

For invalid URLs like abc our Page.navigate method doesn't throw an error. Following a comparison between Chrome and Firefox:

Chrome:

  puppeteer:protocol SEND ► {"sessionId":"AFD670E27BB05E7DCC1D7F8D4B2CEACF","method":"Page.navigate","params":{"url":"asfd","frameId":"E5821CBA23333F71DED2BF5FF64A35E2"},"id":15} +1ms
  puppeteer:protocol ◀ RECV {"error":{"code":-32000,"message":"Cannot navigate to invalid URL"},"id":15,"sessionId":"AFD670E27BB05E7DCC1D7F8D4B2CEACF"} +1ms

Firefox:

puppeteer:protocol SEND ► {"sessionId":1,"method":"Page.navigate","params":{"url":"asfd","frameId":"10"},"id":15} +1ms
1579598516682	RemoteAgent	TRACE	(connection {d1d037a4-51ba-5e4c-b337-a09de75a64ee})-> {"sessionId":1,"method":"Page.navigate","params":{"url":"asfd","frameId":"10"},"id":15}
1579598516686	RemoteAgent	TRACE	<-(connection {d1d037a4-51ba-5e4c-b337-a09de75a64ee}) {"sessionId":1,"id":15,"result":{"frameId":"10"}}

This can easily be fixed by using new URL(url) before calling this.docShell.loadURI(), and wrapping this all into a try/catch for throwing a better error message, which maybe could be similar to what Chrome raises. Therefore we would also have to change the navigation.spec.js test by removing the if/else block. The latter change warrants a pull request on the puppeteer repository even before a patch gets landed in our repository.

I have successfully built Firefox and am looking for some new bugs to work on. Can I work on this?

You are welcome to work on this bug. When you push your patch it will automatically be assigned to you. Let me know if you have questions beside the information from our documentation (see user story field above).

I would also like to work on this is bug.

Flags: needinfo?(hskupin)

(In reply to ashu from comment #3)

I would also like to work on this is bug.

  • this bug

There can only be a single assignee, and thepower asked before. So you may want to find a different bug to get started. Maybe bug 1590451 is something for you?

Flags: needinfo?(hskupin)

Ok, but I am in middle of submitting a patch

Please be respectful with other contributors, especially when we don't know about their current state yet. You can keep your patch for now, just in case theprover won't be able to finish it, or give an update within a week.

I was not trying to be disrespectful, I am sorry it felt that way. I have already moved to another bug

This will be fixed in Bug 1599257

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Component: CDP: Page → CDP
You need to log in before you can comment on or make changes to this bug.