Closed
Bug 1509449
Opened 2 years ago
Closed 2 years ago
Use document.getElementById instead of document.querySelector in ContentBlocking.init
Categories
(Firefox :: Protections UI, enhancement, P3)
Firefox
Protections UI
Tracking
()
RESOLVED
FIXED
Firefox 65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: dao, Assigned: rishabhjairath, Mentored)
Details
(Keywords: good-first-bug, perf, Whiteboard: [lang=js])
Attachments
(1 file, 1 obsolete file)
ContentBlocking.init has this helper function: https://searchfox.org/mozilla-central/rev/876022232b15425bb9efde189caf747823b39567/browser/base/content/browser-contentblocking.js#305 We only ever call this with an id selector, so this will be more efficient if we use getElementById instead of querySelector, and change the calls from $("#whatever") to $("whatever")... i.e. remove #.
Comment hidden (obsolete) |
Assignee | ||
Comment 2•2 years ago
|
||
Hi Dao, I'm a newbie with 2 contributions. Would like to take up this issue
Reporter | ||
Comment 3•2 years ago
|
||
(In reply to rishabhjairath from comment #2) > Hi Dao, I'm a newbie with 2 contributions. Would like to take up this issue Go ahead, let me know if you have questions.
Assignee: nobody → rishabhjairath
Assignee | ||
Comment 4•2 years ago
|
||
I wasn't able to figure out the functionality of browser-contentblocking.js. Could you give me an overview of the functionality? Also do all js files in mozilla-central/browser/base/content get combined to a single js file like it happens with a webpack?
Assignee | ||
Comment 5•2 years ago
|
||
https://phabricator.services.mozilla.com/D13744 Hey dao, could you review my changes please?
Assignee | ||
Comment 6•2 years ago
|
||
Reporter | ||
Comment 7•2 years ago
|
||
Did you see my review in phabricator?
Flags: needinfo?(rishabhjairath)
Hello Dao, I would like to take up this issue.
Flags: needinfo?(dao+bmo)
Reporter | ||
Comment 9•2 years ago
|
||
(In reply to Michael from comment #8) > Hello Dao, I would like to take up this issue. Rishabh is already working on this.
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 10•2 years ago
|
||
Assignee | ||
Comment 11•2 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #7) > Did you see my review in phabricator? https://phabricator.services.mozilla.com/D14009 Before making the requested changes, I did 'hg pull --update' and 'hg checkout default' I couldn't do 'hg commit --amend'. So created a new patch. Can you please tell me the correct order I should follow for editing the original submitted patch.
Flags: needinfo?(rishabhjairath)
Updated•2 years ago
|
Attachment #9029489 -
Attachment is obsolete: true
Comment 12•2 years ago
|
||
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ebc66c250b56 Use document.getElementById instead of document.querySelector in ContentBlocking.init r=dao
Reporter | ||
Comment 13•2 years ago
|
||
(In reply to rishabhjairath from comment #11) > (In reply to Dão Gottwald [::dao] from comment #7) > > Did you see my review in phabricator? > > https://phabricator.services.mozilla.com/D14009 > > Before making the requested changes, I did 'hg pull --update' and 'hg > checkout default' > I couldn't do 'hg commit --amend'. So created a new patch. Can you please > tell me the correct order I should follow for editing the original submitted > patch. I think the problem is that you did hg pull --update with local commits. I think hg pull --rebase is what you want there. I'm not sure what hg checkout does, never used it...
Comment 14•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ebc66c250b56
Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
You need to log in
before you can comment on or make changes to this bug.
Description
•