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

Support Whale extensions store #124

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

theseanl
Copy link

Whale browser is based on Chromium, is run by Naver, and has a noticeable market share of %6 in South Korea. Similar to Edge, it supports installing extensions from CWS but also from its own store https://store.whale.naver.com/extensions/. Would you consider adding support for this store in crxviewer? I tried to update references to regexes throughout the sources with appropriate regex for whale web store. Also I've fixed a match pattern that wasn't working for edge add-ons store.

This also fixes a match pattern for Edge add-ons store.
@Rob--W
Copy link
Owner

Rob--W commented Aug 24, 2022

I'd welcome support for Whale. For future reference and testing, could you paste relevant links/examples of the pages to visit and the actions to take in order to verify that support works? Basically the test plan that verifies that (1) context menu works (2) extension button works (3) the viewer works as expected.

The current PR is missing the logic for preferring the slug in crxviewer.js; for some relevant changes, see the commit that introduced support for the Edge gallery (dbc8a54 from #101).

@theseanl
Copy link
Author

theseanl commented Aug 24, 2022

Steps for testing

Here are what I've verified as working:

  1. Visit an example extension items page https://store.whale.naver.com/detail/fakkbgbaojmoojmlgddealabpahehofh
  2. Verify that the extension's icon is shown (active).
  3. Open context menu, verify that the extension's item is shown.
  4. Click on the context menu item, verify that the extension's source loads in a new tab.
  5. Click on the extension's icon in the toolbar, verify that 2 buttons are shown and are working.
  6. Open the extension's viewer crxviewer.html, paste the url of the above example extension/
  7. Click on the "Open in this viewer" button, verify that the extension's source loads.

AFAIK direct link to the "whx" (analogous to crx) are not exposed to the user-facing website.

Slug?

The current PR is missing the logic for preferring the slug in crxviewer.js; for some relevant changes, see the commit that introduced support for the Edge gallery (dbc8a54 from https://github.com/Rob--W/crxviewer/pull/101).

I cannot find a reference of what "slug" means in this context. I guess it is some internal lingo used in AMO, could you elaborate?

If you are referring to the line 1577 of crxviewer.js in the commit, it is replaced with a call to is_webstore_url in cws_patterns.js in HEAD, which is patched in this PR.

Optional permissions

Currently, the extension has the following host permissions, presumably to bypass CORS header in CWS.

"*://clients2.google.com/service/update2/crx*",
"*://clients2.googleusercontent.com/crx/download/*",

Why not add the following permissions as well?

"*://edge.microsoft.com/extensionwebstorebase/v1/crx*",
"*://msedgeextensions.f.tlu.dl.delivery.mp.microsoft.com/*",
"*://store.whale.naver.com/update/whx*",
"*://store-whale.pstatic.net/update/whx/*"

It will allow downloading of the extension from Edge Add-ons store and Whale store without granting <all_urls> permission.
From how the url of edge add-ons redirection is formed, this probably differ from user to user, but it seems likely that it will be stable for Whale.

@Rob--W
Copy link
Owner

Rob--W commented Aug 24, 2022

I'll have to take a closer look at this PR, but will probably not get to that this week. I did a cursory look, and found \/ in a string literal where it should not be, but overall it seems fine. I'll check in more detail later.

AFAIK direct link to the "whx" (analogous to crx) are not exposed to the user-facing website.

What do you mean by that? In Chrome, crx cannot be installed by clicking links, but right-click + Save-as would work. A link/URL to something that is very likely an extension package (.whx file extension?) is likely a good candidate for support in crxviewer.

Slug?

The current PR is missing the logic for preferring the slug in crxviewer.js; for some relevant changes, see the commit that introduced support for the Edge gallery (dbc8a54 from https://github.com/Rob--W/crxviewer/pull/101).

I cannot find a reference of what "slug" means in this context. I guess it is some internal lingo used in AMO, could you elaborate?

A "URL slug" is generic terminology that refers to the human-readable part of a URL. E.g. in the Chrome Web Store, it is the "chrome-extension-source-v" part in https://chrome.google.com/webstore/detail/chrome-extension-source-v/jifpbeccnghkjeaalbbjmodiffmgedin

If you are referring to the line 1577 of crxviewer.js in the commit, it is replaced with a call to is_webstore_url in cws_patterns.js in HEAD, which is patched in this PR.

I see, you're right. It has been almost two years since that commit.

Optional permissions

Currently, the extension has the following host permissions, presumably to bypass CORS header in CWS.

"*://clients2.google.com/service/update2/crx*",
"*://clients2.googleusercontent.com/crx/download/*",

Why not add the following permissions as well?

"*://edge.microsoft.com/extensionwebstorebase/v1/crx*",
"*://msedgeextensions.f.tlu.dl.delivery.mp.microsoft.com/*",
"*://store.whale.naver.com/update/whx*",
"*://store-whale.pstatic.net/update/whx/*"

It will allow downloading of the extension from Edge Add-ons store and Whale store without granting <all_urls> permission. From how the url of edge add-ons redirection is formed, this probably differ from user to user, but it seems likely that it will be stable for Whale.

Adding new extension permissions disables the extension on update, until the user has approved the new permissions. That's why I don't add new permissions.

@theseanl
Copy link
Author

theseanl commented Aug 24, 2022

What do you mean by that? In Chrome, crx cannot be installed by clicking links, but right-click + Save-as would work. A link/URL to something that is very likely an extension package (.whx file extension?) is likely a good candidate for support in crxviewer.

If there was such a link, I would have provided it in the test steps too, because the input form in crxviewer.html has a description stating that it supports such links. What I meant is that there is no such link (other than those interpolated from update url), so no additional step was provided.

Adding new extension permissions disables the extension on update, until the user has approved the new permissions. That's why I don't add new permissions.

Not granting <all_urls> does provide additional value. When <all_urls> permission is granted, the browser icon is shown in every tab, losing its ability of signaling extension's support on the current tab. At least the current HEAD behaves in this way - It could be a different issue though. Since this extension is a developer tool, I'd argue that it is more reasonable to optimize for real values rather than user-friendliness, etc.

@theseanl
Copy link
Author

theseanl commented Sep 11, 2022

Still haven't got time to review this?
I noticed that there are several places in README.md and crxviewer.html which lists web stores that the extension supports. Users of Whale extensions will be very minor, so I feel it doesn't need to make into such places, but if it should, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants