Remove Document.GetAnonymousElementByAttribute
Categories
(Core :: XBL, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox72 | --- | fixed |
People
(Reporter: bgrins, Assigned: bgrins)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
text/x-phabricator-request
|
Details |
I still see a few consumers in tree, these must be dead code based on https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/dom/base/Document.cpp#7956.
Here are hits: https://searchfox.org/mozilla-central/search?q=GetAnonymousElementByAttribute&path=
Updated•5 years ago
|
Updated•5 years ago
|
Comment 1•5 years ago
|
||
This is more complicated than it sounds. In removing potential callers, I found references in Rust:
https://searchfox.org/mozilla-central/rev/2a355e56c490768be676689de18629485c9d699b/testing/geckodriver/src/marionette.rs#1543
I don't know Rust (yet). I need help with that part of this.
Comment 2•5 years ago
|
||
Comment 3•5 years ago
|
||
Comment on attachment 9104134 [details]
Bug 1591145, remove document.getAnonymousElementByAttribute, part 1.
feedback: I'm looking for a tryserver push with the full test suite. I really want to get an idea of what this patch breaks, particularly in two areas:
(1) The aforementioned Rust code that may need changes
(2) I added in some throw statements to JS test code, specifically to tell me if that code needs an alternate, or can just be removed.
Updated•5 years ago
|
Comment 4•5 years ago
|
||
needinfo from smaug per his request for a tryserver run.
Comment 5•5 years ago
|
||
Whoops. In writing my patch for this, I forgot to check comm-central. In that repository, suite/ is chock-full of calls to this method. mail/ also has a couple...
jorgk, any suggestions?
Comment 6•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5da02997db5ea69d2fe53744ec5e3005ef2f048a
That isn't really all but should cover most of the test.
Comment 7•5 years ago
|
||
hmm, but I should have looked at the patch some more before pushing to try.
Doesn't make much sense to push when it has still several FIXMEs. Oh well.
Comment 8•5 years ago
|
||
(In reply to Alex Vincent [:WeirdAl] from comment #5)
Whoops. In writing my patch for this, I forgot to check comm-central. In that repository, suite/ is chock-full of calls to this method. mail/ also has a couple...
Thanks for the heads-up. This is part of the de-XBL effort that Magnus is leading for Thunderbird. Looks like we have "a couple" (two) call sites left:
https://searchfox.org/comm-central/search?q=GetAnonymousElementByAttribute&case=false®exp=false&path=mail%2F
Over to Magnus.
SeaMonkey/Suite sadly has fallen behind the de-overlay and de-XBL effort and has been non-functional for a while.
Updated•5 years ago
|
Comment 9•5 years ago
|
||
Removing the Thunderbird ones in bug 1591361.
Comment 10•5 years ago
|
||
Comment on attachment 9104134 [details]
Bug 1591145, remove document.getAnonymousElementByAttribute, part 1.
Oops, didn't clear this when I pushed the patch to try
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
Mark, I see some consumers of the getAnonymousElementByAttribute API (which now doesn't do anything) in toolkit/mozapps/extensions/test/browser/browser_interaction_telemetry.js (see the list at https://searchfox.org/mozilla-central/search?q=getAnonymousElementByAttribute&path=). That makes me think at least part of that test isn't working as expected. Could you help me figure out what's going on with it and if those calls should be removed or replaced with something else?
Assignee | ||
Comment 12•5 years ago
|
||
Matt, I see a consumer of getAnonymousElementByAttribute API (which now doesn't do anything) in UITour.jsm at: https://searchfox.org/mozilla-central/rev/3300072e993ae05d50d5c63d815260367eaf9179/browser/components/uitour/UITour.jsm#207. Should that searchPrefsLink target be removed or updated?
Comment 13•5 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #12)
Matt, I see a consumer of getAnonymousElementByAttribute API (which now doesn't do anything) in UITour.jsm at: https://searchfox.org/mozilla-central/rev/3300072e993ae05d50d5c63d815260367eaf9179/browser/components/uitour/UITour.jsm#207. Should that searchPrefsLink target be removed or updated?
That's already addressed by the patch: https://phabricator.services.mozilla.com/D50563#change-IkudWfwBJwKb
Someone on the search team would have more input into whether that code should stick around but I already had seen the deletion in the patch and hoped that someone there was already consulted.
Assignee | ||
Comment 14•5 years ago
|
||
(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #13)
(In reply to Brian Grinstead [:bgrins] from comment #12)
Matt, I see a consumer of getAnonymousElementByAttribute API (which now doesn't do anything) in UITour.jsm at: https://searchfox.org/mozilla-central/rev/3300072e993ae05d50d5c63d815260367eaf9179/browser/components/uitour/UITour.jsm#207. Should that searchPrefsLink target be removed or updated?
That's already addressed by the patch: https://phabricator.services.mozilla.com/D50563#change-IkudWfwBJwKb
Someone on the search team would have more input into whether that code should stick around but I already had seen the deletion in the patch and hoped that someone there was already consulted.
That patch is being split up into component parts and not landed on it's own, so I'm asking here for clarification. Mak, could you confirm if we should remove searchPrefsLink from UITour or update it to not rely on the XBL anonymous content? FWIW this has been broken for a while as it would have stopped working when PopupSearchAutoComplete was moved away from XBL.
Assignee | ||
Comment 15•5 years ago
|
||
document.getAnonymousElementByAttribute is dead code now, and even when it wasn't this would
have returned null ever since <toolbarbutton> stopped using XBL.
Updated•5 years ago
|
Comment 16•5 years ago
|
||
My apologies to everyone, but I had to walk away from this bug due to other commitments that I have. I overestimated the amount of time I would be able to work on this. That's why :bgrins is asking all these questions.
Comment 17•5 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #14)
(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #13)
Mak, could you confirm if we should remove searchPrefsLink from UITour or update it to not rely on the XBL anonymous content? FWIW this has been broken for a while as it would have stopped working when PopupSearchAutoComplete was moved away from XBL.
It looks like this UITour target was pointing to the search settings button on the new one-off buttons, when they were initially introduced on the search bar. The search bar is no more our primary SAP, so it wouldn't make sense to continue doing that. If we should ever need this for the urlbar, we'll just add a new target, for now I don't think it's necessary, one-off buttons were introduced quite some time ago.
Feel free to remove it.
Assignee | ||
Comment 18•5 years ago
|
||
Updated•5 years ago
|
Comment 19•5 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #11)
Mark, I see some consumers of the getAnonymousElementByAttribute API (which now doesn't do anything) in toolkit/mozapps/extensions/test/browser/browser_interaction_telemetry.js (see the list at https://searchfox.org/mozilla-central/search?q=getAnonymousElementByAttribute&path=). That makes me think at least part of that test isn't working as expected. Could you help me figure out what's going on with it and if those calls should be removed or replaced with something else?
This is dead test code related to the old XUL about:addons. I filed bug 1594082 to remove it.
Assignee | ||
Comment 20•5 years ago
|
||
(In reply to Mark Striemer [:mstriemer] from comment #19)
(In reply to Brian Grinstead [:bgrins] from comment #11)
Mark, I see some consumers of the getAnonymousElementByAttribute API (which now doesn't do anything) in toolkit/mozapps/extensions/test/browser/browser_interaction_telemetry.js (see the list at https://searchfox.org/mozilla-central/search?q=getAnonymousElementByAttribute&path=). That makes me think at least part of that test isn't working as expected. Could you help me figure out what's going on with it and if those calls should be removed or replaced with something else?
This is dead test code related to the old XUL about:addons. I filed bug 1594082 to remove it.
Thanks! Will track that here.
Comment 21•5 years ago
|
||
Comment on attachment 9106383 [details]
Bug 1591145 - Remove unused searchPrefsLink UI Tour target
Revision D51741 was moved to bug 1594110. Setting attachment 9106383 [details] to obsolete.
Comment 22•5 years ago
|
||
Comment on attachment 9106357 [details]
Bug 1591145 - Fix lookup of toolbarbutton-icon in TabsList
Revision D51727 was moved to bug 1594123. Setting attachment 9106357 [details] to obsolete.
Assignee | ||
Comment 23•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 24•5 years ago
|
||
Comment 25•5 years ago
|
||
bugherder |
Description
•