remove const Ci.nsIWhatever pattern from Javascript modules in PSM
Categories
(Core :: Security: PSM, defect, P3)
Tracking
()
People
(Reporter: dipen, Assigned: carolina.jimenez.g)
Details
(Whiteboard: [psm-cleanup])
Attachments
(12 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:61.0) Gecko/20100101 Firefox/61.0 Build ID: 20180704003137 Steps to reproduce: Patters such as these are prevalent in Javascript code in PSM. const nsIWhatever = Ci.nsIWhatever const nsWhatever = "mozilla.org/whatever;1" This is actually an anti-pattern and should just use Ci.nsIWhatever and "mozilla.org/whatever;1" directly.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
I would also like to know if I can run something to check anything is broken
Assignee | ||
Comment 2•5 years ago
•
|
||
Hello Dana, On Tuesday I wrote some emails to the persons listed here https://wiki.mozilla.org/Modules/All and only 2 of them answered (for the moment)... I need someone to review the changes on the browser, accessible, extensions, mobile, testing and toolkit modules... I don't know where else can I reach them.
Also, the changes on Phabricator are only like the half of what I wanted to fix, but I didn't push more of them because the revision is already too large. Would you like me to fix the rest and put it on the same revision link? Should I do it in another revision? Or shouldn't I finish the fix?
Thank you
I would give people a little bit more time to respond, and then we'll see what we can do. Also, for changes that affect many parts of the tree, sometimes it can be easier to either break up the patch into multiple patches that each address one module (so there's only one reviewer per patch) or to even have one bug per module. The nice thing about these changes is they don't have to land all at once - you can fix one module more or less independently of the others (in fact, when I asked Dipen to file this bug, I was really only intending to fix this for files in security/ ). Long story short, I think it would be fine to amend the current patch to only include changes that have been reviewed as of now and file follow-up bugs to address the rest of the changes to be made (each in the appropriate component) (if you decide to go this route, we should also file a meta-bug that tracks each of the other bugs).
Assignee | ||
Comment 4•5 years ago
|
||
Hello Danna, I split some of my commits, did rebase, and now I left in the revission D25686 only the files related with security, layout and devtools. I think I didn't break anything.
I will upload the rest I had for each module in differents revissions, all of them "tagged" with this bug.
Assignee | ||
Comment 5•5 years ago
|
||
Bug 1480925 - Removes more anti-patterns related with toolkit/
Bug 1480925 - Removes some eslint problems in toolkit/
Assignee | ||
Comment 6•5 years ago
|
||
Bug 1480925 - Removes more anti-patterns related with browser/
Bug 1480925 - Removes some eslint problems in browser/
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Assignee | ||
Comment 10•5 years ago
|
||
Assignee | ||
Comment 11•5 years ago
|
||
Assignee | ||
Comment 12•5 years ago
|
||
Assignee | ||
Comment 13•5 years ago
|
||
Assignee | ||
Comment 14•5 years ago
|
||
Assignee | ||
Comment 15•5 years ago
|
||
Comment 16•5 years ago
|
||
Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fefc86a2f630 Removes anti-patterns related with Ci.nsIWhatever. r=keeler,yzen
Comment 17•5 years ago
|
||
bugherder |
Assignee | ||
Updated•5 years ago
|
Comment 18•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:keeler, maybe it's time to close this bug?
Let's limit the scope of this back to PSM and call it fixed.
Updated•4 years ago
|
Updated•4 years ago
|
Description
•