Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal inconsistency: runtime.getContexts() proposal differs from Chromium implementation #498

Open
bershanskiy opened this issue Nov 27, 2023 · 3 comments
Labels
inconsistency Inconsistent behavior across browsers proposal Proposal for a change or new feature

Comments

@bershanskiy
Copy link
Member

bershanskiy commented Nov 27, 2023

The proposal document New API: runtime.getContexts() was merged before the first implementation arrived in Chromium. The final implemented API differs from the proposal a little bit. Is there any interest in updating the proposal (for future API implementors and API consumers)?

The differences (non-exhaustive list):

  • the proposal specifies that the ContextFilter argument in runtime.getContexts() is optional (and omitting it is equivalent to passing an empty object {}) while in reality the argument is mandatory (docs, discussion).
  • extension.ContextType.SIDE_PANEL does not have a declared value (it should be just SIDE_PANEL, obviously)

The under-specified behaviors (non-exhaustive list):

@xeenon xeenon added inconsistency Inconsistent behavior across browsers proposal Proposal for a change or new feature and removed needs-triage labels Dec 7, 2023
@Rob--W Rob--W added the follow-up: chrome Needs a response from a Chrome representative label Dec 7, 2023
@Rob--W
Copy link
Member

Rob--W commented Apr 25, 2024

@oliverdunk Any thoughts on this one? We are actively working on the implementation in Firefox, at https://bugzilla.mozilla.org/show_bug.cgi?id=1875480

I'm especially wondering about why options_ui is missing from getContexts, since it is present in getViews(). And what is the expected contextType for options_ui?

It is not really a tab, so maybe a new context type may make sense here, e.g. "OPTIONS_TAB" (open to other names).

@oliverdunk
Copy link
Member

I spoke to @rdcronin about each of the questions here.

the proposal specifies that the ContextFilter argument in runtime.getContexts() is optional (and omitting it is equivalent to passing an empty object {}) while in reality the argument is mandatory

We couldn't find this noted anywhere, but made this mandatory because we believed it was the preference from Mozilla's side. If @Rob--W is happy to make it optional, we would be open to that.

extension.ContextType.SIDE_PANEL does not have a declared value (it should be just SIDE_PANEL, obviously)

I've opened a PR to fix this here: #650. It is a non-goal to keep the proposals up to date with future changes, but this seemed like an easy enough fix.

do extension popups (browser actions) have an associated window and/or tab?

I didn't get a chance to chat to Devlin about this one, but I think that makes sense, at least in Chromium where they close on tab changes. Curious for what other browser vendors think.

do extension sidebar frames have an associated window and/or tab? Does associated tab id depend on sidebar binding to a particular tab?

In Chromium, they are always tied to a window, so we should fix https://bugs.chromium.org/p/chromium/issues/detail?id=1468914 and return a window ID. For the tab ID, our preference would be only including that for tab-specific side panels.

do extensions options frames embedded into Chrome settings (docs) show up in getContexts()? Currently, Chromium does not report them.

We do think we should start including them. Our preference would be to simply use the TAB context type for both the options page and options UI, since we couldn't think of any realistic situations where you would need to behave differently based on an open context being an options UI. If there are situations we've missed though we are open to chatting more about this.

@oliverdunk oliverdunk removed the follow-up: chrome Needs a response from a Chrome representative label Jul 1, 2024
@Rob--W
Copy link
Member

Rob--W commented Jul 1, 2024

I spoke to @rdcronin about each of the questions here.

the proposal specifies that the ContextFilter argument in runtime.getContexts() is optional (and omitting it is equivalent to passing an empty object {}) while in reality the argument is mandatory

We couldn't find this noted anywhere, but made this mandatory because we believed it was the preference from Mozilla's side. If @Rob--W is happy to make it optional, we would be open to that.

The idea behind requiring an option is to encourage extension developers to think about the filter they are using, so that they would use a very specific filter to avoid returning too many, if the API ever returns more.

do extension popups (browser actions) have an associated window and/or tab?

I didn't get a chance to chat to Devlin about this one, but I think that makes sense, at least in Chromium where they close on tab changes. Curious for what other browser vendors think.

We set the windowId but not the tabId because the popup is anchored to a window, not a tab. Open to consider non--1 tabId if it makes sense.

do extensions options frames embedded into Chrome settings (docs) show up in getContexts()? Currently, Chromium does not report them.

We do think we should start including them. Our preference would be to simply use the TAB context type for both the options page and options UI, since we couldn't think of any realistic situations where you would need to behave differently based on an open context being an options UI. If there are situations we've missed though we are open to chatting more about this.

In Firefox we use "TAB" for contexts in embedded options pages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Inconsistent behavior across browsers proposal Proposal for a change or new feature
Projects
None yet
Development

No branches or pull requests

4 participants