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)

Product:
Firefox
Component:
Protections UI
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 65
Tracking Flags:
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)

Bug 1509449 - Use document.getElementById instead of document.querySelector in ContentBlocking.init
47 bytes, text/x-phabricator-request
Details | Review 
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 #.
Hi Dao, I'm a newbie with 2 contributions. Would like to take up this issue
(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
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?
https://phabricator.services.mozilla.com/D13744
Hey dao, could you review my changes please?
Did you see my review in phabricator?
Flags: needinfo?(rishabhjairath)
Hello Dao, I would like to take up this issue.
Flags: needinfo?(dao+bmo)
(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)
(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)
Attachment #9029489 - Attachment is obsolete: true
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
(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...
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.