-
Notifications
You must be signed in to change notification settings - Fork 330
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
Swap innerHTML to document.createFragment for pageAction/popup #2491
Conversation
@maxxcrawford What do you think of completely removing pageAction right now? See #2477. |
6e5e3cc
to
bf2859c
Compare
3ca33f4
to
ec80c78
Compare
b91d1f2
to
ec80c78
Compare
…EN_IN_PICKER functions
ec80c78
to
49d6db7
Compare
tableData.appendChild(spanMenuText); | ||
tableRowMenuItem.appendChild(tableData); | ||
tableMenu.appendChild(tableRowMenuItem); | ||
divNewContainer.appendChild(tableMenu); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At L1236, we need to use divNewContainer.replaceChildren(tableMenu);
to avoid creating a duplicate of the new container item everytime we enter MANAGE_CONTAINERS_PICKER panel.
Closing right now to stop blocking @dannycolin on other stuff. |
Summary
When submitting to AMO, there's a warning about using
innerHTML
.Blocking L10N PR:mozilla-l10n/multi-account-containers-l10n#22This PR replaces all instances of it.
TODO:
Testing
This should have no visible changes in the UI. Check both the popup panel and the pageAction (in the URL bar). Both should appear/match
main
.Screenshots
Confirmed with VPN set that the flag setting works/displays as expected:
![image](https://user-images.githubusercontent.com/2692333/214078933-b15590f2-553d-4275-a618-d41cf0c2e8ab.png)