fix: Fixed Chrome Not Opening Extensions Pages provided in URL

[kodergeek]

[Rob--W]

Do you need anything else from me to proceed? I posted a very brief comment in reply to your question at https://github.com/mozilla/web-ext/issues/1979#issuecomment-924761488. It does answer your question, but I can imagine if some more detail is needed to understand my reply.

[kodergeek]

Yes, I appreciate if you can point in code where I should add the call so that bg.js extension can successfully open the extensions tab as you described in your comment. I couldn't figure it out and I tried in a few different places in code to no avail.

[Rob--W]

If your goal is to use the WebSocket to transmit the message, then you'll have to send a message once the manager extension has connected.

Here is an example of a snippet that connects: https://github.com/mozilla/web-ext/blob/6.4.0/src/extension-runners/chromium.js#L147

You would have to send the message once (not on every connect), to prevent the extension from opening tabs again when the WebSocket connection is temporarily interrupted for some reason.

For the current requirements, a simpler alternative is to hard-code the startup URL in the source of the manager extension. After all, the URLs are known when the extension is generated, and they need to be opened only once. To implement this approach, you'd need to pass the list of URLs to https://github.com/mozilla/web-ext/blob/6.4.0/src/extension-runners/chromium.js#L154 and write it to the generated bg.js

To make sure that the tabs are opened only once, use chrome.runtime.onInstalled and open the tabs if details.reason === "install".

In your current patch you're also filtering on chrome://extensions specificially. I suggest to just do this for any chrome: URL. E.g. chrome://version would have the same issue.

[kodergeek]

I have now updated the PR with the second approach as suggested by you, eg hard-coding the startup URL in the source of the manager extension.

I have also tested it with some different URL combinations and it seems to be working. Please check it out and let me know if there's anything that should be improved.

Also, there are 4 failing checks for my PR which I have no idea what they relate to. So if you think the fix is good to be merged I'd appreciate if you could advise me how to fix the failing checks :)

[Rob--W]

The test failures are linting issues. Run eslint to see the exact errors.

This is the output of circleci:

/home/circleci/web-ext/src/extension-runners/chromium.js
  163:34  error  Unexpected string concatenation                      prefer-template
  163:52  error  Strings must use singlequote                         quotes
  163:55  error  Missing semicolon                                    semi
  164:36  error  Missing semicolon                                    semi
  174:1   error  This line has a length of 95. Maximum allowed is 80  max-len
  294:1   error  This line has a length of 84. Maximum allowed is 80  max-len

✖ 6 problems (6 errors, 0 warnings)
  4 errors and 0 warnings potentially fixable with the `--fix` option.

[codecov[bot]]

Codecov Report

Merging #2321 (8ca4550) into master (1a0c926) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head 8ca4550 differs from pull request most recent head 7b56d10. Consider uploading reports for the commit 7b56d10 to get more accurate results

@@           Coverage Diff           @@
##           master    #2321   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files          32       32           
  Lines        1699     1707    +8     
=======================================
+ Hits         1697     1705    +8     
  Misses          2        2           

Impacted Files	Coverage Δ
src/extension-runners/chromium.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1a0c926...7b56d10. Read the comment docs.

[kodergeek]

Thanks for your feedback @Rob--W. I have now resolved the issues you mentioned and have also added a unit test to both check for presence of special URLs in generated script, and also lack of them in the args list sent to chrome.

One issue I encountered during running existing unit tests was with the ordering of arguments being pushed to chromeFlags array. I had to move some code up to be able to check starting URLs and send them to createReloadManagerExtension because createReloadManagerExtension is now expecting the special arguments list, and it resulted in reordering of chromeFlags array because startingUrls was pushed first in chromeFlags array, and it resulted in 2 unit tests failing. So I have rearranged the code slightly and it has caused a slight code duplication at here and here. But no unit tests are failing as a result.

[Rob--W]

The code duplication is really unfortunate. Can you think of a way to implement the functionality without duplication?

[kodergeek]

Well yes the easiest way to remove code duplication and keeping the functionality is to make a small change to 2 unit tests by shifting around the list of arguments that are being asserted. I have updated the PR with the suggested fix. Is that something that is acceptable or I should not change the existing unit tests at all?

[kodergeek]

Hi @Rob--W Just wondering if you've had time to review this?

[Rob--W]

Hi @Rob--W Just wondering if you've had time to review this?

Unfortunately I did not. I'll try to get back to you next week.

[dleetr]

This PR seems to be abandoned. I need the functionality, so I have continued it in the link below, preserving the original work here and adding a small enhancement to it. https://github.com/mozilla/web-ext/pull/2774