1956945 - "webExtension.uninstall" with an emtpy string as id shouldn't raise an "unknown error"

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Currently our webExtension.uninstall command returns a unknown failure when trying to uninstall a web extension with a value of an empty string. Instead we should return a no such webextension error:

 0:19.28 pid:21790 1743110409213	RemoteAgent	DEBUG	WebDriverBiDiConnection 5f507b0f-ad68-4a19-8fda-aef8ff0ce771 -> {"id":7,"method":"webExtension.uninstall","params":{"extension":""}}
 0:19.28 pid:21790 1743110409213	RemoteAgent	TRACE	Received command webExtension.uninstall for destination ROOT
 0:19.28 pid:21790 1743110409213	RemoteAgent	DEBUG	WebDriverBiDiConnection 5f507b0f-ad68-4a19-8fda-aef8ff0ce771 <- {"type":"error","id":7,"error":"unknown error","message":"[Exception... \"aID must be a non-empty string\"  nsresult: \"0x80070057 (NS_ERROR_ILLEGAL_VALUE)\"  location: \"JS frame :: resource://gre/modules/AddonManager.sys.mjs :: getAddonByID :: line 2869\"  data: no]","stacktrace":"getAddonByID@resource://gre/modules/AddonManager.sys.mjs:2869:24\ngetAddonByID@resource://gre/modules/AddonManager.sys.mjs:4306:33\nuninstall@chrome://remote/content/shared/Addon.sys.mjs:185:45\nuninstall@chrome://remote/content/webdriver-bidi/modules/root/webExtension.sys.mjs:171:22\nhandleCommand@chrome://remote/content/shared/messagehandler/MessageHandler.sys.mjs:260:33\nexecute@chrome://remote/content/shared/webdriver/Session.sys.mjs:390:32\nonPacket@chrome://remote/content/webdriver-bidi/WebDriverBiDiConnection.sys.mjs:236:37\nonMessage@chrome://remote/content/server/WebSocketTransport.sys.mjs:127:18\nhandleEvent@chrome://remote/content/server/WebSocketTransport.sys.mjs:109:14\n"}

This happens because we pass the id of the webextension directly through to the AddonManager which then fails. We probably have to special-case it and check if it's empty first.

Better would be to maybe enforce that in the CDDL but it looks like that empty strings are not supported for the JSON variant.

When we fix that some wdspec tests have to be updated as well.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There is also the following metadata file that needs an update or as of this state getting removed given that the test will then pass with the requested changes.

https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/testing/web-platform/meta/webdriver/tests/bidi/web_extension/uninstall/invalid.py.ini

Comment 2

[Ben Chatterton [:benchatt]]

Attachment: Bug 1956945 - [webdriver-bidi] Improve "webExtension.uninstall" command for empty extension id r=whimboo!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e13967666b5
[webdriver-bidi] Improve "webExtension.uninstall" command for empty extension id r=whimboo,webdriver-reviewers

Comment 4

[amarc]

https://hg.mozilla.org/mozilla-central/rev/7e13967666b5

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/52301 for changes under testing/web-platform/tests

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot