1437230 - Remove feed binding and build DOM using JS

Status: RESOLVED FIXED

(Firefox :: Page Info Window, task)

Description

[Tim Nguyen :ntim]

https://raw.githubusercontent.com/mozilla/gecko-dev/master/browser/base/content/pageinfo/feeds.xml

It's used for the "Feeds" tab of the page info window, but this looks small enough it probably doesn't need its own binding.

Comment 1

[Tim Nguyen :ntim]

Attachment: Bug 1437230 - Remove feed binding and build DOM using JS.

Review commit: https://reviewboard.mozilla.org/r/221158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/221158/

Comment 2

[Code Review Bot [:reviewbot]]

Comment on attachment 8951876 [details]
Bug 1437230 - Remove feed binding and build DOM using JS.

https://reviewboard.mozilla.org/r/221158/#review227040


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/base/content/pageinfo/feeds.js:54
(Diff revision 1)
> +
> +  const subscribeButton = document.createElement("button");
> +  subscribeButton.className = "feed-subscribe";
> +  subscribeButton.addEventListener("click",
> +    () => openUILinkIn(url, "current", { ignoreAlt: true }));
> +  subscribeButton.setAttribute("label", gBundle.getString("feedSubscribe"));

Error: 'gbundle' is not defined. [eslint: no-undef]

::: browser/base/content/pageinfo/feeds.js:55
(Diff revision 1)
> +  const subscribeButton = document.createElement("button");
> +  subscribeButton.className = "feed-subscribe";
> +  subscribeButton.addEventListener("click",
> +    () => openUILinkIn(url, "current", { ignoreAlt: true }));
> +  subscribeButton.setAttribute("label", gBundle.getString("feedSubscribe"));
> +  subscribeButton.setAttribute("accesskey", gBundle.getString("feedSubscribe.accesskey"));

Error: 'gbundle' is not defined. [eslint: no-undef]

Comment 3

[Tim Nguyen :ntim]

Comment on attachment 8951876 [details]
Bug 1437230 - Remove feed binding and build DOM using JS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/221158/diff/1-2/

Comment 4

[:Gijs (he/him)]

Comment on attachment 8951876 [details]
Bug 1437230 - Remove feed binding and build DOM using JS.

https://reviewboard.mozilla.org/r/221158/#review227160

Looks OK - because you've already done the work I guess we can do this, but I suspect this entire pane can just go away.

Note that browser/base/content/test/pageinfo/browser_pageInfo.js is bright orange in the trypush, so this can't land as-is.

Comment 5

[Tim Nguyen :ntim]

Comment on attachment 8951876 [details]
Bug 1437230 - Remove feed binding and build DOM using JS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/221158/diff/2-3/

Comment 6

[:Gijs (he/him)]

Comment on attachment 8951876 [details]
Bug 1437230 - Remove feed binding and build DOM using JS.

https://reviewboard.mozilla.org/r/221158/#review227170

rs=me assuing try is happier.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75e8d75c3de9
Remove feed binding and build DOM using JS. r=Gijs

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/75e8d75c3de9