Skip to content

Update for "ban image" functionality (#1516)#1517

Merged
extesy merged 5 commits intoextesy:masterfrom
GrosPoulet:master
Feb 23, 2025
Merged

Update for "ban image" functionality (#1516)#1517
extesy merged 5 commits intoextesy:masterfrom
GrosPoulet:master

Conversation

@GrosPoulet
Copy link
Copy Markdown
Contributor

  • when an image (or video & audio track) is banned:
    • url of banned image is stored in background page local storage
    • a message is sent to all tabs with updated list of banned urls
  • removal of async call to background page: check for banned image is now performed synchronously in each tab

- when an image (or video & audio track) is banned:
   - url of banned image is stored in background page local storage
   - a message is sent to all tabs with updated list of banned urls
- removal of async call to background page: check for banned image is now performed synchronously in each tab
@GrosPoulet GrosPoulet closed this Jan 5, 2025
@GrosPoulet GrosPoulet reopened this Jan 5, 2025
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 5, 2025

@GrosPoulet
Copy link
Copy Markdown
Contributor Author

@extesy looks like i have lost the ability to merge my PR ???

Comment thread js/background.js Outdated
Comment thread js/hoverzoom.js Outdated
@extesy
Copy link
Copy Markdown
Owner

extesy commented Jan 6, 2025

Yes, I wanted to have opportunity to review the code before it's merged instead of doing post-merge fixes.

@extesy
Copy link
Copy Markdown
Owner

extesy commented Feb 16, 2025

@GrosPoulet If you still want this PR to be merged, please resolve conflicts. I'm going to be making more changes for Manifest V3 compatibility, so if it's not merged soon then it will be progressively harder to resolve conflicts.

Comment thread js/hoverzoom.js
Comment on lines +2648 to +2651
function isImageBanned(url) {
if (!url) return false;
reutrn !!bannedImagesList[url];
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@GrosPoulet Is bannedImagesList an array (list) or a set? According to https://stackoverflow.com/questions/2430000/determine-if-string-is-in-list-in-javascript if it's a list then you should use .include to check membership, and if it's a set then you should use .has.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@extesy bannedImagesList is a set (key:value), urls are used as keys

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

key:value is a dictionary (aka a map), not a set. Are values used for anything?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

value = location (page url) where banned image was found

@extesy extesy merged commit bbf8eab into extesy:master Feb 23, 2025
@sonarqubecloud
Copy link
Copy Markdown

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.

2 participants