Closed Bug 1763124 Opened 2 years ago Closed 2 years ago

Handle navigation to error pages for browsingContext.navigate

Categories

(Remote Protocol :: WebDriver BiDi, task, P3)

task
Points:
5

Tracking

(firefox102 fixed)

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: jdescottes, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [bidi-m3-mvp], [wptsync upstream])

Attachments

(3 files)

Follow up to handle more specific navigation use cases.
We should support navigation to error pages, such as about:neterror

Whiteboard: [webdriver:backlog]

As discussed we will try to have a look next week as long as we are working on navigation in M3.

Points: --- → 5
Priority: -- → P3
Whiteboard: [webdriver:backlog] → [bidi-m3-mvp]

Note that the WebProgress listener supports the LOCATION_CHANGE_ERROR_PAGE flag for onLocationChange notifications:
https://searchfox.org/mozilla-central/rev/da6a85e615827d353e5ca0e05770d8d346b761a9/uriloader/base/nsIWebProgressListener.idl#431-443

Assignee: nobody → hskupin
Status: NEW → ASSIGNED

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)

Note that the WebProgress listener supports the LOCATION_CHANGE_ERROR_PAGE flag for onLocationChange notifications:
https://searchfox.org/mozilla-central/rev/da6a85e615827d353e5ca0e05770d8d346b761a9/uriloader/base/nsIWebProgressListener.idl#431-443

This is actually not the right solution. Instead we should still check a possible failure state when the STATE_STOP state change is notified:
https://searchfox.org/mozilla-central/rev/7f729f601c0b738f870ae0ed49098f9268e250f9/uriloader/base/nsIWebProgressListener.idl#375-387

That way we should also be able to return details of the error to the client similar to what Marionette does:

[task 2022-05-19T11:34:04.132Z] 11:34:04 INFO - 1652960044132 Marionette DEBUG 5 <- [1,38,{"error":"unknown error","message":"Reached error page: about:neterror?e=unknownProtocolFound&u=thisprotocoldoesnotexis ... /EventEmitter.jsm:160:20\nreceiveMessage@chrome://remote/content/marionette/actors/MarionetteEventsParent.jsm:44:25\n"},null]

In XPCOM we have very details error messages:
https://searchfox.org/mozilla-central/source/js/xpconnect/src/xpc.msg#132

It would be great to use the appropriate one with the error message as returned to the client. Sadly there is only ChromeUtils.getXPCOMErrorName() but nothing for the message. After speaking to Smaug it might make sense to add a new method here that would allow to return the error message instead.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #4)

It would be great to use the appropriate one with the error message as returned to the client. Sadly there is only ChromeUtils.getXPCOMErrorName() but nothing for the message. After speaking to Smaug it might make sense to add a new method here that would allow to return the error message instead.

Nika, do you know if there is already a way to get these messages from JS modules? If not would ChromeUtils.cpp be a good place for that as well? Thanks.

Flags: needinfo?(nika)

It looks like there are cases when a navigation doesn't start at all and as such also no state change notification is sent. Instead I only see a location change notification, so we would still have to use the flag as mentioned in comment 2.

 0:15.47 pid:31003 1652988345090	WebDriver BiDi	DEBUG	89d7cf72-a12d-41a5-aaea-ecfe3a540d38 -> {"id":8,"method":"browsingContext.navigate","params":{"context":"2a7edbee-d43e-4fec-aba2-9ae54bcfbee4","url":"http://localhost:0","wait":"complete"}}
 0:15.47 pid:31003 1652988345090	RemoteAgent	TRACE	Received command browsingContext.navigate for destination ROOT
 0:15.47 pid:31003 1652988345090	RemoteAgent	TRACE	Received command browsingContext._getBaseURL for destination WINDOW_GLOBAL
 0:15.47 pid:31003 1652988345090	RemoteAgent	TRACE	Created MessageHandler WINDOW_GLOBAL for session 89d7cf72-a12d-41a5-aaea-ecfe3a540d38
 0:15.47 pid:31003 1652988345090	RemoteAgent	TRACE	Received command browsingContext._getBaseURL for destination WINDOW_GLOBAL
 0:15.47 pid:31003 1652988345090	RemoteAgent	TRACE	Module windowglobal/browsingContext.jsm found for WINDOW_GLOBAL
 0:15.47 pid:31003 [Parent 31005: Main Thread]: I/BCWebProgress OnLocationChange({isTopLevel:1, isLoadingDocument:0}, {URI:http://localhost:0/, originalURI:http://localhost:0/}, http://localhost:0/, LOCATION_CHANGE_ERROR_PAGE) on {top:1, id:30, url:http://localhost:0/}
 0:15.47 pid:31003 1652988345097	RemoteAgent	TRACE	MessageHandler WINDOW_GLOBAL for session 89d7cf72-a12d-41a5-aaea-ecfe3a540d38 is being destroyed
 0:15.47 pid:31003 1652988345097	RemoteAgent	TRACE	Unregistered MessageHandler WINDOW_GLOBAL for session 89d7cf72-a12d-41a5-aaea-ecfe3a540d38
 0:15.47 pid:31003 [Parent 31005: Main Thread]: I/BCWebProgress OnSecurityChange(<null>, <null>, 4) on {top:1, id:30, url:http://localhost:0/}
 0:43.07 TEST_END: TIMEOUT, expected OK

I'm not aware of a convenient way to get these messages right now. For now, the main way they're used is when throwing an XPCOM exception across XPConnect IIRC. That file is also what is used to populate Cr[...] but the message isn't exposed there.

I'm generally fine with adding something to chromeutils if you need this for whatever reason I suppose? If you do take that approach, we should probably fetch it with nsXPCException::NameAndFormatForNSResult (https://searchfox.org/mozilla-central/rev/9b43f4743a4d1ffa8e9da4defe2675daa363a5cb/js/xpconnect/src/xpcprivate.h#1909-1910) rather than building out a new table.

Flags: needinfo?(nika)

(In reply to Nika Layzell [:nika] (ni? for response) from comment #7)

I'm not aware of a convenient way to get these messages right now. For now, the main way they're used is when throwing an XPCOM exception across XPConnect IIRC. That file is also what is used to populate Cr[...] but the message isn't exposed there.

I'm generally fine with adding something to chromeutils if you need this for whatever reason I suppose? If you do take that approach, we should probably fetch it with nsXPCException::NameAndFormatForNSResult (https://searchfox.org/mozilla-central/rev/9b43f4743a4d1ffa8e9da4defe2675daa363a5cb/js/xpconnect/src/xpcprivate.h#1909-1910) rather than building out a new table.

Thanks Nika. I think for now we should be fine with just the error names given that these explain the failure quite well. If it turns out that we need even the extended data I would come back to your proposal in how to retrieve the message.

Attachment #9277481 - Attachment description: Bug 1763124 - [wdspec] Add tests for errorneous requests for "browsingContext.navigate". → Bug 1763124 - [wdspec] Add tests for erroneous requests for "browsingContext.navigate".
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/500c5063b7d3
[remote] Use Deferred instead of a plain Promise for ProgressListener. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/a2dc6a551dcf
[webdriver-bidi] Support error handling for browsingContext.navigate. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/465c1e1af734
[wdspec] Add tests for erroneous requests for "browsingContext.navigate". r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34181 for changes under testing/web-platform/tests
Whiteboard: [bidi-m3-mvp] → [bidi-m3-mvp], [wptsync upstream]
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
Upstream PR was closed without merging
Upstream PR was closed without merging
Upstream PR merged by whimboo
Regressions: 1779723
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: