Use document.getElementById instead of document.querySelector in ContentBlocking.init
RESOLVED
FIXED
in Firefox 65
Status
()
People
(Reporter: dao, Assigned: rishabhjairath, Mentored)
Tracking
({good-first-bug, perf})
Firefox Tracking Flags
(firefox65 fixed)
Details
(Whiteboard: [lang=js])
Attachments
(1 attachment, 1 obsolete attachment)
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•4 months ago
|
||
Hi Dao, I'm a newbie with 2 contributions. Would like to take up this issue
|
(Reporter) | |
Comment 3•4 months 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•4 months 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•3 months ago
|
||
https://phabricator.services.mozilla.com/D13744 Hey dao, could you review my changes please?
|
|
(Assignee) | |
Comment 6•3 months ago
|
||
|
|
(Reporter) | |
Comment 7•3 months 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•3 months 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•3 months ago
|
||
|
|
(Assignee) | |
Comment 11•3 months 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•3 months ago
|
Attachment #9029489 -
Attachment is obsolete: true
|
||
Comment 12•3 months 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•3 months 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•3 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/ebc66c250b56
Status: NEW → RESOLVED
Last Resolved: 3 months 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
•