1509449 - Use document.getElementById instead of document.querySelector in ContentBlocking.init

Status: RESOLVED FIXED

(Firefox :: Protections UI, enhancement, P3)

Description

[Dão Gottwald [:dao]]

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 1

[Hrushikesh Bodas]

Hello. I would like to take up this issue.

Comment 2

[rishabhjairath]

Hi Dao, I'm a newbie with 2 contributions. Would like to take up this issue

Comment 3

[Dão Gottwald [:dao]]

(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.

Comment 4

[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?

Comment 5

[rishabhjairath]

https://phabricator.services.mozilla.com/D13744
Hey dao, could you review my changes please?

Comment 6

[rishabhjairath]

Attachment: Bug 1509449 - Use document.getElementById instead of document.querySelector in ContentBlocking.init

Comment 7

[Dão Gottwald [:dao]]

Did you see my review in phabricator?

Comment 8

[Michael]

Hello Dao, I would like to take up this issue.

Comment 9

[Dão Gottwald [:dao]]

(In reply to Michael from comment #8)
> Hello Dao, I would like to take up this issue.

Rishabh is already working on this.

Comment 10

[rishabhjairath]

Attachment: Bug 1509449 - Use document.getElementById instead of document.querySelector in ContentBlocking.init

Comment 11

[rishabhjairath]

(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.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebc66c250b56
Use document.getElementById instead of document.querySelector in ContentBlocking.init r=dao

Comment 13

[Dão Gottwald [: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...

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/ebc66c250b56