1584122 - clean out some xbl related functionality: getAnonymousElementByAttribute, getAnonymousNodes

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Magnus Melin [:mkmelin]]

Some xbl related functionality is going away in bug 1584019. We need to remove the usage of getAnonymousElementByAttribute, getAnonymousNodes

Comment 1

[Jorg K (CEST = GMT+2)]

https://searchfox.org/comm-central/search?q=GetAnonymousElementByAttribute&case=false&regexp=false&path=
Disregarding gData, there are four callsites in mail(news) and some more in test (scroll down).
That's begs the question when we will drop gData so we don't get hits on its code anymore.

https://searchfox.org/comm-central/search?q=getAnonymousNodes&case=false&regexp=false&path=
Few hits, including in tests (scroll down).

Comment 2

[Magnus Melin [:mkmelin]]

I have a patch amost finished, will send to try soon. Philpp said he'll be ready for gdata removal tomorrow.

Won't be updating the gdata ones, especially as it's not exactly clear how (without debugging it).

Comment 3

[Richard Marti (:Paenglab)]

Does this also include https://searchfox.org/comm-central/search?q=anonid&case=false&regexp=false&path= ?

Comment 4

[Magnus Melin [:mkmelin]]

anonid attribute will be just like any other random attribute, that you can match on. I think we're already covered there.

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: bug1584122_xbl_cleanup.patch

Try has just the expected errors: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f304dfb138d184906577e3656c17160c3baa367b

Comment 6

[Paul Morris [:pmorris]]

Comment on attachment 9096355 [details] [diff] [review]
bug1584122_xbl_cleanup.patch

Review of attachment 9096355 [details] [diff] [review]:
-----------------------------------------------------------------

Great to be clearing out the dregs of XBL.  r+ with the comments addressed.  Tested like so:

- Right-clicked on the column headers for a table of messages and select an option, the columns show/hide as expected, but the popup menu does not go away like I expected.  I checked and this is already what happens without this patch.  We should file a separate bug.

- Not sure exactly how to test removing an activity element.  Clearing the list in the activity manager worked as expected.

- Checked that "Reply with Template" was not an option when creating a filter when there were no templates saved, and that it was an option after creating a template.

::: mail/components/activity/content/activity.js
@@ +159,5 @@
>            item.remove();
>            break;
>          }
>        } else {
> +        let actElement = item.querySelector("[actID='" + aID + "']");

A template string might be clearer here:  `[actID="${aID}"]`

::: mail/test/mozmill/shared-modules/WindowHelpers.jsm
@@ -1043,5 @@
> -    aid: function _get_anon_elementid(aId, aQuery) {
> -      return new elib.Elem(this.a(aId, aQuery));
> -    },
> -
> -    /**

Hooray for being able to delete this code!

::: mailnews/base/search/content/searchWidgets.js
@@ +111,5 @@
> +      );
> +      for (let { label, value } of raMenulist.findTemplates()) {
> +        menulist.appendItem(label, value);
> +      }
> +      raMenulist.findTemplates(true, menulist);

I see that `findTemplates` no longer takes arguments, which makes me suspect that this line is just a leftover and should be removed.  Or am I missing something?

@@ +1393,5 @@
>        /**
>         * Check if there exist any templates in this account.
>         *
> +       * @param {boolean} populateTemplateList - Whether to add menuitems
> +       *  for the found templates.

This param has been removed, so its doc string should go as well.

Comment 7

[Jorg K (CEST = GMT+2)]

Testing "checkin-needed". Please ignore.

Comment 8

[Magnus Melin [:mkmelin]]

Attachment: bug1584122_xbl_cleanup.patch

Thanks!

Comment 9

[Magnus Melin [:mkmelin]]

(In reply to Paul Morris [:pmorris] from comment #6)

• Right-clicked on the column headers for a table of messages and select an
  option, the columns show/hide as expected, but the popup menu does not go
  away like I expected. I checked and this is already what happens without
  this patch. We should file a separate bug.

It goes away once you click outside it. I think it works like designed, so you can choose which columns to show (many changes at once).

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/aab0650cbf1d
clean out some xbl related functionality: getAnonymousElementByAttribute, getAnonymousNodes. r=pmorris

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 9096990 [details] [diff] [review]
bug1584122_xbl_cleanup.patch

We just need to backport https://hg.mozilla.org/comm-central/rev/aab0650cbf1d#l8.46 to fix bug 1584835 if we do TB 70 beta 4.

Comment 12

[Jorg K (CEST = GMT+2)]

TB 70 beta 3:
https://hg.mozilla.org/releases/comm-beta/rev/ca79441535b96bc867ddb5b7f9a66c66c4b46d1b
https://hg.mozilla.org/releases/comm-beta/rev/c0aec565926568e418d44e269083e373b8a2b6de

Comment 13

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/comm-beta/rev/47ae0b1a94434a05537a440ca3e94dc8fe4507d5