Page.navigate doesn't throw error for invalid URLs
Categories
(Remote Protocol :: CDP, task, P3)
Tracking
(Not tracked)
People
(Reporter: whimboo, Unassigned, Mentored)
References
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?
|
Reporter | |
Comment 2•4 years ago
|
||
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).
|
|
Reporter | |
Comment 5•4 years ago
|
||
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?
|
|
Reporter | |
Comment 7•4 years ago
|
||
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
|
Assignee | |
Updated•3 years ago
|
Description
•