Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1260983002:
Add the Chrome Playpen Dashboard URL to the manifest file of the Chrome Web Store App. (Closed)
|
Created:
5 years, 4 months ago by markusheintz Modified:
5 years, 4 months ago CC:
chromium-reviews, arv+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd the Chrome Playpen Dashboard URL to the manifest file of the Chrome Web Store App.
BUG=TODO
Patch Set 1 #Patch Set 2 : Increase manifest version number #
Total comments: 1
Messages
Total messages: 8 (1 generated)
markusheintz@chromium.org changed reviewers: + kalman@chromium.org, markusheintz@chromium.org
Could you please review this CL. Thanks a lot
lgtm https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... File chrome/browser/resources/webstore_app/manifest.json (right): https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... chrome/browser/resources/webstore_app/manifest.json:16: "https://chrome.google.com/manage" I didn't realise this is what we agreed on... It's still not quite right, the extent of the webstore does not include "manage", this is basically a workaround for a principled solution[*]. A side effect will be that on ChromeOS if you have a tab to chrome.google.com/manage open, clicking on the webstore app icon will refocus that window. As I also mentioned, the management console doesn't need access to all the webstorePrivate APIs, only that 1. It would be better to pull that out separately. However, fixing it properly will be time consuming. Please reference that in the bug that you will file on this. [*] in which we define a new API, and instead of having an app, we simply expose that API to chrome.google.com. There is a bit of extra work here.
On 2015/07/29 17:48:47, kalman wrote: > lgtm > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > File chrome/browser/resources/webstore_app/manifest.json (right): > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > chrome/browser/resources/webstore_app/manifest.json:16: > "https://chrome.google.com/manage" > I didn't realise this is what we agreed on... > > It's still not quite right, the extent of the webstore does not include > "manage", this is basically a workaround for a principled solution[*]. > > A side effect will be that on ChromeOS if you have a tab to > chrome.google.com/manage open, clicking on the webstore app icon will refocus > that window. > > As I also mentioned, the management console doesn't need access to all the > webstorePrivate APIs, only that 1. It would be better to pull that out > separately. > > However, fixing it properly will be time consuming. Please reference that in the > bug that you will file on this. > > [*] in which we define a new API, and instead of having an app, we simply expose > that API to http://chrome.google.com. There is a bit of extra work here. Hm, I think if we're sure we eventually want to have this in a new API, then we should do that sooner rather than later; certainly before any kind of public release. Otherwise we'll have to keep supporting the "legacy" version for a while (e.g. the CWS would need to call either the old or the new API depending on the Chrome version).
On 2015/07/30 08:52:53, Marc Treib wrote: > On 2015/07/29 17:48:47, kalman wrote: > > lgtm > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > File chrome/browser/resources/webstore_app/manifest.json (right): > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > chrome/browser/resources/webstore_app/manifest.json:16: > > "https://chrome.google.com/manage" > > I didn't realise this is what we agreed on... > > > > It's still not quite right, the extent of the webstore does not include > > "manage", this is basically a workaround for a principled solution[*]. > > > > A side effect will be that on ChromeOS if you have a tab to > > chrome.google.com/manage open, clicking on the webstore app icon will refocus > > that window. > > > > As I also mentioned, the management console doesn't need access to all the > > webstorePrivate APIs, only that 1. It would be better to pull that out > > separately. > > > > However, fixing it properly will be time consuming. Please reference that in > the > > bug that you will file on this. > > > > [*] in which we define a new API, and instead of having an app, we simply > expose > > that API to http://chrome.google.com. There is a bit of extra work here. > > Hm, I think if we're sure we eventually want to have this in a new API, then we > should do that sooner rather than later; certainly before any kind of public > release. Otherwise we'll have to keep supporting the "legacy" version for a > while (e.g. the CWS would need to call either the old or the new API depending > on the Chrome version). I think we have two different challenges: 1) If the Supervised User web dashboard is open in a tab and a user clicks on the Webstore icon, then the Webstore should open rather than the Supervise User dashboard tab being focused. @Ben: Why would this happen? Would the same happen on Desktop? If not why not? @Marc: I would love to reproduce this issue before we head into another direction. Assuming this is an issue I think we need to add our own component App for the Supervised User dashboard in order to fix this. Let's do this first then using the webstorePrivate API. 2) Move the new parts of the webstorePrivate API (added by Marc) to a separate API. This can happen in a second step (I'm all for this!!! But as long as the Supervised User dashboard enjoys that same special security treatment as the CWS we are fine). To me it sounds like we are all in agreement about how this should look like when we are done. Now let's take this step by step. We don't want to stumble and fall :-).
On 2015/07/30 14:12:16, markusheintz_ wrote: > On 2015/07/30 08:52:53, Marc Treib wrote: > > On 2015/07/29 17:48:47, kalman wrote: > > > lgtm > > > > > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > > File chrome/browser/resources/webstore_app/manifest.json (right): > > > > > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > > chrome/browser/resources/webstore_app/manifest.json:16: > > > "https://chrome.google.com/manage" > > > I didn't realise this is what we agreed on... > > > > > > It's still not quite right, the extent of the webstore does not include > > > "manage", this is basically a workaround for a principled solution[*]. > > > > > > A side effect will be that on ChromeOS if you have a tab to > > > chrome.google.com/manage open, clicking on the webstore app icon will > refocus > > > that window. > > > > > > As I also mentioned, the management console doesn't need access to all the > > > webstorePrivate APIs, only that 1. It would be better to pull that out > > > separately. > > > > > > However, fixing it properly will be time consuming. Please reference that in > > the > > > bug that you will file on this. > > > > > > [*] in which we define a new API, and instead of having an app, we simply > > expose > > > that API to http://chrome.google.com. There is a bit of extra work here. > > > > Hm, I think if we're sure we eventually want to have this in a new API, then > we > > should do that sooner rather than later; certainly before any kind of public > > release. Otherwise we'll have to keep supporting the "legacy" version for a > > while (e.g. the CWS would need to call either the old or the new API depending > > on the Chrome version). > > I think we have two different challenges: > > 1) If the Supervised User web dashboard is open in a tab and a user clicks on > the Webstore icon, > then the Webstore should open rather than the Supervise User dashboard tab being > focused. > > @Ben: Why would this happen? Would the same happen on Desktop? If not why not? It's a launcher integration. I don't know whether that extends to the desktop app launcher, you'd need to try it. It's definitely integrated into Chrome OS. > @Marc: I would love to reproduce this issue before we head into another > direction. Assuming this is an issue I think we need to add our own component > App for the Supervised User dashboard in order to fix this. Let's do this first > then using the webstorePrivate API. > > 2) Move the new parts of the webstorePrivate API (added by Marc) to a separate > API. This can happen in a second step (I'm all for this!!! But as long as the > Supervised User dashboard enjoys that same special security treatment as the CWS > we are fine). > > To me it sounds like we are all in agreement about how this should look like > when we are done. > Now let's take this step by step. We don't want to stumble and fall :-).
On 2015/07/30 18:48:13, kalman wrote: > On 2015/07/30 14:12:16, markusheintz_ wrote: > > On 2015/07/30 08:52:53, Marc Treib wrote: > > > On 2015/07/29 17:48:47, kalman wrote: > > > > lgtm > > > > > > > > > > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > > > File chrome/browser/resources/webstore_app/manifest.json (right): > > > > > > > > > > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > > > chrome/browser/resources/webstore_app/manifest.json:16: > > > > "https://chrome.google.com/manage" > > > > I didn't realise this is what we agreed on... > > > > > > > > It's still not quite right, the extent of the webstore does not include > > > > "manage", this is basically a workaround for a principled solution[*]. > > > > > > > > A side effect will be that on ChromeOS if you have a tab to > > > > chrome.google.com/manage open, clicking on the webstore app icon will > > refocus > > > > that window. > > > > > > > > As I also mentioned, the management console doesn't need access to all the > > > > webstorePrivate APIs, only that 1. It would be better to pull that out > > > > separately. > > > > > > > > However, fixing it properly will be time consuming. Please reference that > in > > > the > > > > bug that you will file on this. > > > > > > > > [*] in which we define a new API, and instead of having an app, we simply > > > expose > > > > that API to http://chrome.google.com. There is a bit of extra work here. > > > > > > Hm, I think if we're sure we eventually want to have this in a new API, then > > we > > > should do that sooner rather than later; certainly before any kind of public > > > release. Otherwise we'll have to keep supporting the "legacy" version for a > > > while (e.g. the CWS would need to call either the old or the new API > depending > > > on the Chrome version). > > > > I think we have two different challenges: > > > > 1) If the Supervised User web dashboard is open in a tab and a user clicks on > > the Webstore icon, > > then the Webstore should open rather than the Supervise User dashboard tab > being > > focused. > > > > @Ben: Why would this happen? Would the same happen on Desktop? If not why not? > > It's a launcher integration. I don't know whether that extends to the desktop > app launcher, you'd need to try it. It's definitely integrated into Chrome OS. I tried, it doesn't extend to desktop. > > @Marc: I would love to reproduce this issue before we head into another > > direction. Assuming this is an issue I think we need to add our own component > > App for the Supervised User dashboard in order to fix this. Let's do this > first > > then using the webstorePrivate API. > > > > 2) Move the new parts of the webstorePrivate API (added by Marc) to a separate > > API. This can happen in a second step (I'm all for this!!! But as long as the > > Supervised User dashboard enjoys that same special security treatment as the > CWS > > we are fine). > > > > To me it sounds like we are all in agreement about how this should look like > > when we are done. > > Now let's take this step by step. We don't want to stumble and fall :-).
On 2015/07/31 07:49:10, Marc Treib wrote: > On 2015/07/30 18:48:13, kalman wrote: > > On 2015/07/30 14:12:16, markusheintz_ wrote: > > > On 2015/07/30 08:52:53, Marc Treib wrote: > > > > On 2015/07/29 17:48:47, kalman wrote: > > > > > lgtm > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > > > > File chrome/browser/resources/webstore_app/manifest.json (right): > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/1260983002/diff/20001/chrome/browser/resource... > > > > > chrome/browser/resources/webstore_app/manifest.json:16: > > > > > "https://chrome.google.com/manage" > > > > > I didn't realise this is what we agreed on... > > > > > > > > > > It's still not quite right, the extent of the webstore does not include > > > > > "manage", this is basically a workaround for a principled solution[*]. > > > > > > > > > > A side effect will be that on ChromeOS if you have a tab to > > > > > chrome.google.com/manage open, clicking on the webstore app icon will > > > refocus > > > > > that window. > > > > > > > > > > As I also mentioned, the management console doesn't need access to all > the > > > > > webstorePrivate APIs, only that 1. It would be better to pull that out > > > > > separately. > > > > > > > > > > However, fixing it properly will be time consuming. Please reference > that > > in > > > > the > > > > > bug that you will file on this. > > > > > > > > > > [*] in which we define a new API, and instead of having an app, we > simply > > > > expose > > > > > that API to http://chrome.google.com. There is a bit of extra work here. > > > > > > > > Hm, I think if we're sure we eventually want to have this in a new API, > then > > > we > > > > should do that sooner rather than later; certainly before any kind of > public > > > > release. Otherwise we'll have to keep supporting the "legacy" version for > a > > > > while (e.g. the CWS would need to call either the old or the new API > > depending > > > > on the Chrome version). > > > > > > I think we have two different challenges: > > > > > > 1) If the Supervised User web dashboard is open in a tab and a user clicks > on > > > the Webstore icon, > > > then the Webstore should open rather than the Supervise User dashboard tab > > being > > > focused. > > > > > > @Ben: Why would this happen? Would the same happen on Desktop? If not why > not? > > > > It's a launcher integration. I don't know whether that extends to the desktop > > app launcher, you'd need to try it. It's definitely integrated into Chrome OS. > > I tried, it doesn't extend to desktop. > > > > @Marc: I would love to reproduce this issue before we head into another > > > direction. Assuming this is an issue I think we need to add our own > component > > > App for the Supervised User dashboard in order to fix this. Let's do this > > first > > > then using the webstorePrivate API. > > > > > > 2) Move the new parts of the webstorePrivate API (added by Marc) to a > separate > > > API. This can happen in a second step (I'm all for this!!! But as long as > the > > > Supervised User dashboard enjoys that same special security treatment as the > > CWS > > > we are fine). > > > > > > To me it sounds like we are all in agreement about how this should look like > > > when we are done. > > > Now let's take this step by step. We don't want to stumble and fall :-). Obsolete. Closing this now. |
