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

fix: Limit reloads to enabled extensions in chrome extension runner #2365

Merged
merged 3 commits into from
Dec 15, 2021

Conversation

fregante
Copy link
Contributor

@fregante fregante changed the title Limit reloads to enabled extensions Fix: Limit reloads to enabled extensions Dec 10, 2021
@rpl
Copy link
Member

rpl commented Dec 10, 2021

@fregante this looks definitely like a reasonable short term workaround to fix #2364, thanks for filing the issue and for looking into a fix!

The CI job failure is due to the linting of the commit message. That step of the CI job will validate the commit message that github chooses by default when the PR is squashed and merged, and so:

  • when the pull request has only one commit (like in this case), it checks that the single commit message follows the commit message conventions
  • when the pull request has more than one commit, it checks that the pull request title follows the the commit message conventions

And so for fixing that CI job failure you just need to reword the commit message as "fix: Limit reloads to enabled extensions" (or "fix: Limit reloads to enabled extensions in chrome extension runner" to make it clear which of the extension runner the fix is about) and force push it here.

@fregante
Copy link
Contributor Author

I can't check it out right now, but I added 2 commits that might make it pass 😅

@rpl rpl changed the title Fix: Limit reloads to enabled extensions fix: Limit reloads to enabled extensions in chrome extension runner Dec 10, 2021
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #2365 (3d4dc27) into master (9917f04) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2365   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files          32       32           
  Lines        1701     1701           
=======================================
  Hits         1699     1699           
  Misses          2        2           
Impacted Files Coverage Δ
src/extension-runners/chromium.js 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 9917f04...3d4dc27. Read the comment docs.

@fregante
Copy link
Contributor Author

Demo of this piece of code working locally:

You can see that the extension on the right is reloaded while the disabled one stays disabled:

Screen.Recording.6.mov

Copy link
Member

@rpl rpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fregante Thank you! (also for recording a screencast to quickly let me see the fix in use ❤️ )

@rpl rpl merged commit 1b0494a into mozilla:master Dec 15, 2021
@fregante fregante deleted the patch-2 branch December 15, 2021 12:03
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.

web-ext run should not enable disabled extensions
2 participants