Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
8100009:
Whitelist ChromeVox (Closed)
|
Created:
9 years, 2 months ago by dmazzoni Modified:
9 years, 2 months ago Reviewers:
Finnur, Erik does not do reviews, Mihai Parparita -not on Chrome, commit-bot: I haz the power, Aaron Boodman CC:
chromium-reviews, Erik does not do reviews, mihaip+watch_chromium.org Base URL:
svn://chrome-svn/chrome/trunk/src/ Visibility:
Public. |
DescriptionWhitelist ChromeVox so it can script webui pages.
BUG=98717
TEST=manual testing with ChromeVox from web store
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106683
Patch Set 1 #Patch Set 2 : '' #Messages
Total messages: 15 (0 generated)
I don't think this is what we want. This structure was added for testing and then made obsolete by things like component extensions. We have a bug on hand for removing this function: http://code.google.com/p/chromium/issues/detail?id=78163 Why not use a component function?
Component extension, I mean.
ChromeVox is already a component extension on Chrome OS. It makes sense to bundle it there because users need to have a way to enable accessibility "out of the box". For other platforms, we're in a middle ground: we don't think it's worth bundling it with every Chrome download, but it's definitely useful and some users want to install it. Offering it on the web store makes sense. Unfortunately it's much less useful without having access to webui pages. What do you think? - Dominic On Mon, Oct 3, 2011 at 3:29 AM, <finnur@chromium.org> wrote: > I don't think this is what we want. This structure was added for testing > and > then made obsolete by things like component extensions. We have a bug on > hand > for removing this function: > http://code.google.com/p/**chromium/issues/detail?id=**78163<http://code.goog... > > Why not use a component function? > > http://codereview.chromium.**org/8100009/<http://codereview.chromium.org/8100... >
On 2011/10/03 18:17:36, Dominic Mazzoni wrote: > ChromeVox is already a component extension on Chrome OS. It makes sense to > bundle it there because users need to have a way to enable accessibility > "out of the box". > > For other platforms, we're in a middle ground: we don't think it's worth > bundling it with every Chrome download, but it's definitely useful and some > users want to install it. Offering it on the web store makes sense. > Unfortunately it's much less useful without having access to webui pages. > > What do you think? > > - Dominic > > On Mon, Oct 3, 2011 at 3:29 AM, <mailto:finnur@chromium.org> wrote: > > > I don't think this is what we want. This structure was added for testing > > and > > then made obsolete by things like component extensions. We have a bug on > > hand > > for removing this function: > > > http://code.google.com/p/**chromium/issues/detail?id=**78163%3Chttp://code.go...> > > > > Why not use a component function? > > > > > http://codereview.chromium.**org/8100009/%3Chttp://codereview.chromium.org/81...> > > I am vaguely opposed to this, but not enough to put a foot down. Erik? Mihai? If we do it, there should be tests for it. Are there? Also it seems wrong to be configuring the whitelist inside extension.cc. Seems like someone higher up in the stack should do it, e.g., so that it is easily testable.
Is it a feasible alternative to split up the extension into two? 1. A small component extension that handles scripting of WebUI pages 2. A regular extension (in the store) that has the bulk of the current functioanlity, but uses http://code.google.com/chrome/extensions/messaging.html#external to communicate with the component extension where needed? Another alternative is to use the component updater ( https://docs.google.com/a/google.com/document/d/1gqKdDaoH9s8pbQkb9I_EX8Cfdsjg...) to ship the whole extension as a component extension without impacting the initial download size. Mihai On Mon, Oct 3, 2011 at 2:40 PM, <aa@chromium.org> wrote: > http://codereview.chromium.**org/8100009/<http://codereview.chromium.org/8100... >
Thanks for suggesting some alternatives, that's really helpful. I think splitting the extension could work just fine, and the component extension could probably be quite small. Any idea how to trigger enabling the component extension in the first place, though? It'd probably be best if the component extension wasn't even loaded unless the web store extension was installed and talking to it. - Dominic On Mon, Oct 3, 2011 at 10:38 PM, Mihai Parparita <mihaip@chromium.org>wrote: > Is it a feasible alternative to split up the extension into two? > 1. A small component extension that handles scripting of WebUI pages > 2. A regular extension (in the store) that has the bulk of the current > functioanlity, but uses > http://code.google.com/chrome/extensions/messaging.html#external to > communicate with the component extension where needed? > > Another alternative is to use the component updater ( > https://docs.google.com/a/google.com/document/d/1gqKdDaoH9s8pbQkb9I_EX8Cfdsjg...) > to ship the whole extension as a component extension without impacting the > initial download size. > > Mihai > > On Mon, Oct 3, 2011 at 2:40 PM, <aa@chromium.org> wrote: > >> http://codereview.chromium.**org/8100009/<http://codereview.chromium.org/8100... >> > >
Is this not a problem on Chrome OS, because there's a checkbox on the settings page that enables/disables the component extension? Do you just need to get at the text/DOM of WebUI pages, or do you need to actually run script on them? Another alternative that Anthony and Ben came up with is to add a separate extension API for getting at the contents of a tab (but not otherwise run scripts or modify the contents). This API we would conceivably allow to run on all pages, even WebUI ones, since it won't have any security implications. Mihai On Wed, Oct 5, 2011 at 8:05 AM, Dominic Mazzoni <dmazzoni@chromium.org>wrote: > Thanks for suggesting some alternatives, that's really helpful. I think > splitting the extension could work just fine, and the component extension > could probably be quite small. Any idea how to trigger enabling the > component extension in the first place, though? It'd probably be best if the > component extension wasn't even loaded unless the web store extension was > installed and talking to it. > > - Dominic > > > On Mon, Oct 3, 2011 at 10:38 PM, Mihai Parparita <mihaip@chromium.org>wrote: > >> Is it a feasible alternative to split up the extension into two? >> 1. A small component extension that handles scripting of WebUI pages >> 2. A regular extension (in the store) that has the bulk of the current >> functioanlity, but uses >> http://code.google.com/chrome/extensions/messaging.html#external to >> communicate with the component extension where needed? >> >> Another alternative is to use the component updater ( >> https://docs.google.com/a/google.com/document/d/1gqKdDaoH9s8pbQkb9I_EX8Cfdsjg...) >> to ship the whole extension as a component extension without impacting the >> initial download size. >> >> Mihai >> >> On Mon, Oct 3, 2011 at 2:40 PM, <aa@chromium.org> wrote: >> >>> http://codereview.chromium.**org/8100009/<http://codereview.chromium.org/8100... >>> >> >> >
On Wed, Oct 5, 2011 at 5:16 PM, Mihai Parparita <mihaip@chromium.org> wrote: > Is this not a problem on Chrome OS, because there's a checkbox on the > settings page that enables/disables the component extension? That's correct. Also, there's a distinction because on Chrome OS this is the *only* accessibility solution, so it makes sense to tightly integrate it. On Mac & Windows our first priority is supporting third-party accessibility solutions, and we're doing a decent job there - but we'd like to offer users the choice of using ChromeVox, which supports a number of unique features. > Do you just need to get at the text/DOM of WebUI pages, or do you need to > actually run script on them? Another alternative that Anthony and Ben came > up with is to add a separate extension API for getting at the contents of a > tab (but not otherwise run scripts or modify the contents). This API we > would conceivably allow to run on all pages, even WebUI ones, since it won't > have any security implications. > Unfortunately, we need more than just the text/DOM - for example, we need to let the user use a keypress to simulate a click on a control. I do like the idea of coming up with an extension API that would give us just what's necessary and no more, because that would enable third-party developers to build accessibility solutions, too. - Dominic Mihai > > > On Wed, Oct 5, 2011 at 8:05 AM, Dominic Mazzoni <dmazzoni@chromium.org>wrote: > >> Thanks for suggesting some alternatives, that's really helpful. I think >> splitting the extension could work just fine, and the component extension >> could probably be quite small. Any idea how to trigger enabling the >> component extension in the first place, though? It'd probably be best if the >> component extension wasn't even loaded unless the web store extension was >> installed and talking to it. >> >> - Dominic >> >> >> On Mon, Oct 3, 2011 at 10:38 PM, Mihai Parparita <mihaip@chromium.org>wrote: >> >>> Is it a feasible alternative to split up the extension into two? >>> 1. A small component extension that handles scripting of WebUI pages >>> 2. A regular extension (in the store) that has the bulk of the current >>> functioanlity, but uses >>> http://code.google.com/chrome/extensions/messaging.html#external to >>> communicate with the component extension where needed? >>> >>> Another alternative is to use the component updater ( >>> https://docs.google.com/a/google.com/document/d/1gqKdDaoH9s8pbQkb9I_EX8Cfdsjg...) >>> to ship the whole extension as a component extension without impacting the >>> initial download size. >>> >>> Mihai >>> >>> On Mon, Oct 3, 2011 at 2:40 PM, <aa@chromium.org> wrote: >>> >>>> http://codereview.chromium.**org/8100009/<http://codereview.chromium.org/8100... >>>> >>> >>> >> >
On Thu, Oct 6, 2011 at 4:38 PM, Dominic Mazzoni <dmazzoni@chromium.org>wrote: > I we would conceivably allow to run on all pages, even WebUI ones, since it >> won't have any security implications. >> > > Unfortunately, we need more than just the text/DOM - for example, we need > to let the user use a keypress to simulate a click on a control. > > I do like the idea of coming up with an extension API that would give us > just what's necessary and no more, because that would enable third-party > developers to build accessibility solutions, too. > The ability to simulate events came up for the offscreen tabs API too ( http://www.chromium.org/developers/design-documents/extensions/offscreen-tabs...), it might make sense to break that out into its own API. Mihai
Re-opening this issue based on our discussion; I updated the comments to reflect that this whitelist is a temporary solution until we have an extension API that allows any extension to request read-only access to webui pages.
Sigh, ok. At least there are tests for it. lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/8100009/11001
Change committed as 106683 |
