1422934 - Remove the "toolbarbutton-image" binding and migrate consumers of it to the "toolbarbutton" binding

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, task, P3)

Description

[Brian Grinstead [:bgrins]]

The "toolbarbutton-image" binding provides custom content to generate simpler markup inside a toolbarbutton. There's only one consumer of "toolbarbutton-image" in m-c ("tabbrowser-close-tab-button").

I believe we can just update the consumer to use "toolbarbutton", as the .toobarbutton-text element will be empty and shouldn't take up any space. This can be confirmed via mozscreenshots or manual testing.

- https://dxr.mozilla.org/mozilla-central/search?q=toolbarbutton-image&redirect=true
- https://dxr.mozilla.org/mozilla-central/rev/709f355a7a8c4ae426d1824841a71ffdb5ce0137/toolkit/content/widgets/toolbarbutton.xml#78
- https://dxr.mozilla.org/mozilla-central/rev/709f355a7a8c4ae426d1824841a71ffdb5ce0137/browser/base/content/tabbrowser.xml#7785
- https://dxr.mozilla.org/mozilla-central/search?q=path%3Atoolbarbutton&redirect=false

Comment 1

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

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

Comment 2

[Ian Moody [:Kwan] (UTC+1)]

Attachment: Bug 1422934 - Remove unused toolbarbutton-image binding.

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

Comment 3

[Brian Grinstead [:bgrins]]

Comment on attachment 8936599 [details]
Bug 1422934 - Remove unused toolbarbutton-image binding.

https://reviewboard.mozilla.org/r/207352/#review213180

Redirecting review to Gijs for an extra sanity check since I proposed this approach

Comment 4

[Ian Moody [:Kwan] (UTC+1)]

As suspected the .toolbarbutton-text (and .toolbarbutton-multiline-text) don't take up any space.

One could keep the old behaviour by moving the <content/> from toolbarbutton-image to tabbrowser-close-tab-button, but given that changing between icons, text, and icons + text modes for toolbars seems like it probably went away with australis, I'm not sure there is any point.

Comment 5

[:Gijs (out for now; he/him)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

https://reviewboard.mozilla.org/r/207350/#review213226

Tentative r+, see below.

::: browser/base/content/tabbrowser.xml:7739
(Diff revision 1)
>         This binding relies on the structure of the tabbrowser binding.
>         Therefore it should only be used as a child of the tab or the tabs
>         element (in both cases, when they are anonymous nodes of <tabbrowser>).
>    -->
>    <binding id="tabbrowser-close-tab-button"
> -           extends="chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image">
> +           extends="chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton">

I mean, this does mean that given a window with 100 tabs we now create 200 extra nodes, right? That seems kind of suboptimal... but I also don't really have a much better idea. Plus if there are a lot of tabs then the tab button is hidden except on the selected tab, so maaaaybe we (can?) avoid constructing all the nodes immediately? I dunno if we do or don't... but perhaps it's worth just doing some performance checking here?

Comment 6

[:Gijs (out for now; he/him)]

Comment on attachment 8936599 [details]
Bug 1422934 - Remove unused toolbarbutton-image binding.

https://reviewboard.mozilla.org/r/207352/#review213228

Comment 7

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

https://reviewboard.mozilla.org/r/207350/#review213240

::: browser/base/content/tabbrowser.xml:7739
(Diff revision 1)
>         This binding relies on the structure of the tabbrowser binding.
>         Therefore it should only be used as a child of the tab or the tabs
>         element (in both cases, when they are anonymous nodes of <tabbrowser>).
>    -->
>    <binding id="tabbrowser-close-tab-button"
> -           extends="chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image">
> +           extends="chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton">

Ah, right, that'd be the other reason to consider moving <content/> to tabbrowser-close-tab-button.  I did wonder about the performance implications of the extra nodes but forgot to mention it.

Comment 8

[:Gijs (out for now; he/him)]

I mean, if we don't already, we could try if it's OK to mark the node as -moz-binding: none while the tab is not selected and close buttons on non-selected tabs are hidden. That should avoid the node creation (including the image!) assuming that styling kicks in "in time". cf. bug 1374691.

Comment 9

[Ian Moody [:Kwan] (UTC+1)]

(In reply to :Gijs from comment #8)
> I mean, if we don't already, we could try if it's OK to mark the node as
> -moz-binding: none while the tab is not selected and close buttons on
> non-selected tabs are hidden. That should avoid the node creation (including
> the image!) assuming that styling kicks in "in time". cf. bug 1374691.

Unfortunately it doesn't seem to, the browser toolbox still shows those nodes as there.

I'm assuming talos chrome is the suite that'd be useful for this, in which case restoring the content optimisation doesn't seem to make much difference:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=a5c4ff436ae0&newProject=try&newRevision=637632d28b2a6189cada377fa6a3838e73d0d926&framework=1

(assuming there's actually something in there that'd test this meaningfully)

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #9)
> (In reply to :Gijs from comment #8)
> > I mean, if we don't already, we could try if it's OK to mark the node as
> > -moz-binding: none while the tab is not selected and close buttons on
> > non-selected tabs are hidden. That should avoid the node creation (including
> > the image!) assuming that styling kicks in "in time". cf. bug 1374691.
> 
> Unfortunately it doesn't seem to, the browser toolbox still shows those
> nodes as there.

The Browser Toolbox doesn't always get the right mutations inside of XBL anonymous content, so if you've already expanded one of the nodes before the -moz-binding: none was set they may not be removed. Can you double check this is the case by waiting to inspect the node until after the binding is set to none, or by running `document.getAnonymousNodes($0)` after selecting the element in the inspector?

Comment 11

[Brian Grinstead [:bgrins]]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #9)
> I'm assuming talos chrome is the suite that'd be useful for this, in which
> case restoring the content optimisation doesn't seem to make much difference:
> https://treeherder.mozilla.org/perf.html#/
> compare?originalProject=try&originalRevision=a5c4ff436ae0&newProject=try&newR
> evision=637632d28b2a6189cada377fa6a3838e73d0d926&framework=1
> 
> (assuming there's actually something in there that'd test this meaningfully)

I would run all of the talos suites (and not just chrome) to be sure - but yes I would say if Talos doesn't catch a regression then it should generally be OK to land.

Comment 12

[Ian Moody [:Kwan] (UTC+1)]

(In reply to Brian Grinstead [:bgrins] from comment #10)
> (In reply to Ian Moody [:Kwan] (UTC+0) from comment #9)
> > (In reply to :Gijs from comment #8)
> > > I mean, if we don't already, we could try if it's OK to mark the node as
> > > -moz-binding: none while the tab is not selected and close buttons on
> > > non-selected tabs are hidden. That should avoid the node creation (including
> > > the image!) assuming that styling kicks in "in time". cf. bug 1374691.
> > 
> > Unfortunately it doesn't seem to, the browser toolbox still shows those
> > nodes as there.
> 
> The Browser Toolbox doesn't always get the right mutations inside of XBL
> anonymous content, so if you've already expanded one of the nodes before the
> -moz-binding: none was set they may not be removed. Can you double check
> this is the case by waiting to inspect the node until after the binding is
> set to none, or by running `document.getAnonymousNodes($0)` after selecting
> the element in the inspector?

I did wonder about the toolbox not handling XUL/XBL 100%, so my test was to get the tab in such a state, then open the toolbox and inspect.  I just tried getAnonymousNodes() to double check as well and it does indeed return all three nodes.

(In reply to Brian Grinstead [:bgrins] from comment #11)
> (In reply to Ian Moody [:Kwan] (UTC+0) from comment #9)
> > I'm assuming talos chrome is the suite that'd be useful for this, in which
> > case restoring the content optimisation doesn't seem to make much difference:
> > https://treeherder.mozilla.org/perf.html#/
> > compare?originalProject=try&originalRevision=a5c4ff436ae0&newProject=try&newR
> > evision=637632d28b2a6189cada377fa6a3838e73d0d926&framework=1
> > 
> > (assuming there's actually something in there that'd test this meaningfully)
> 
> I would run all of the talos suites (and not just chrome) to be sure - but
> yes I would say if Talos doesn't catch a regression then it should generally
> be OK to land.

Okay, I'll try a -t all run.

(I take it not including the <content/> in toolbar-close-tab-button is preferable if there's no performance regression since it reduces the XBL LoC?)

Comment 13

[Ian Moody [:Kwan] (UTC+1)]

While I don't have any experience interpreting talos results, nothing in this seems obviously concerning:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=93b69e696f01b79f6a7bb1252bef87381f655c26&framework=1&filter=linux%20opt&selectedTimeRange=172800

(The few large changes seem within the error margins anyway)

Second opinion?

Comment 14

[Dão Gottwald [:dao]]

I'm not sure that we actually need the close button to be a <toolbarbutton>. For instance, the soundplaying / mute thingy is just an <image>.

Comment 15

[Ian Moody [:Kwan] (UTC+1)]

(In reply to Dão Gottwald [::dao] from comment #14)
> I'm not sure that we actually need the close button to be a <toolbarbutton>.
> For instance, the soundplaying / mute thingy is just an <image>.

This does seem to work by the way (junking the extends entirely and switching to <xul:image> at https://dxr.mozilla.org/mozilla-central/rev/5d203926da51a7e949a20818664b19d5b115572d/browser/base/content/tabbrowser.xml#7816 ), at least from cursory testing, haven't tried running tests.

Comment 16

[Brian Grinstead [:bgrins]]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #15)
> (In reply to Dão Gottwald [::dao] from comment #14)
> > I'm not sure that we actually need the close button to be a <toolbarbutton>.
> > For instance, the soundplaying / mute thingy is just an <image>.
> 
> This does seem to work by the way (junking the extends entirely and
> switching to <xul:image> at
> https://dxr.mozilla.org/mozilla-central/rev/
> 5d203926da51a7e949a20818664b19d5b115572d/browser/base/content/tabbrowser.
> xml#7816 ), at least from cursory testing, haven't tried running tests.

Either way would work for me. I don't see any Talos issues with your current patches.

Comment 17

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

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

Comment 18

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936599 [details]
Bug 1422934 - Remove unused toolbarbutton-image binding.

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

Comment 19

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

Try seems fine with the image change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51f8cb8187054201578d5c92b0a13e18a75aa4bc&selectedJob=152744768
(though bug 1425879 is annoying)

And though bug 1381817, bug 1414741, and bug 1381430, make mozscreenshots oh-so-fun right now, I think the only real difference is about a pixel more of tab title is shown before the fadeout:
https://screenshots.mattn.ca/comparisons/try/fe552e288c5fbca5df96b7d6eda9714fbf41d977/try/cdad026ab6d833ac224c6725709bfff2e14fef15/osx-10-10/primaryUI_205_tabsInTitlebar_twoPinnedWithOverflow_maximized_allToolbars_compactLight_compactDensity.png

Comment 20

[:Gijs (out for now; he/him)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

https://reviewboard.mozilla.org/r/207350/#review215016

Yeah, this wfm. Thanks for the thorough work here!

Comment 21

[Ian Moody [:Kwan] (UTC+1)]

Thanks Gijs!

Sheriff, try run is in comment #19, though that's pre-histedit, so change three is now change one and old-one is gone (and 4 was another bug).

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/123d69b3834a
Make the tab close button an image, and remove the extends from its binding. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/b97013a21476
Remove unused toolbarbutton-image binding. r=Gijs

Comment 23

[Dão Gottwald [:dao]]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #19)
> And though bug 1381817, bug 1414741, and bug 1381430, make mozscreenshots
> oh-so-fun right now, I think the only real difference is about a pixel more
> of tab title is shown before the fadeout:
> https://screenshots.mattn.ca/comparisons/try/
> fe552e288c5fbca5df96b7d6eda9714fbf41d977/try/
> cdad026ab6d833ac224c6725709bfff2e14fef15/osx-10-10/
> primaryUI_205_tabsInTitlebar_twoPinnedWithOverflow_maximized_allToolbars_comp
> actLight_compactDensity.png

Is this Mac-only? It might fix bug 1415671 then...

If it wasn't Mac-only, I'd like to better understand what's causing the difference, as this is one of the primary UI pieces where we care about such minor stuff.

Comment 24

[Andreea Pavel [:apavel]]

Backed out for failing e.g. testing/firefox-ui/tests/puppeteer/test_tabbar.py r=backout on a CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b97013a2147601ed4464172e3fa493c33f543d50

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=153111029&repo=autoland&lineNumber=22759

https://treeherder.mozilla.org/logviewer.html#?job_id=153111640&repo=autoland&lineNumber=7621

Backout: https://hg.mozilla.org/integration/autoland/rev/39f703c43a6b353d6525f6db4c1ad6324487b22c

Comment 25

[Ian Moody [:Kwan] (UTC+1)]

(In reply to Dão Gottwald [::dao] from comment #23)
> (In reply to Ian Moody [:Kwan] (UTC+0) from comment #19)
> > And though bug 1381817, bug 1414741, and bug 1381430, make mozscreenshots
> > oh-so-fun right now, I think the only real difference is about a pixel more
> > of tab title is shown before the fadeout:
> > https://screenshots.mattn.ca/comparisons/try/
> > fe552e288c5fbca5df96b7d6eda9714fbf41d977/try/
> > cdad026ab6d833ac224c6725709bfff2e14fef15/osx-10-10/
> > primaryUI_205_tabsInTitlebar_twoPinnedWithOverflow_maximized_allToolbars_comp
> > actLight_compactDensity.png
> 
> Is this Mac-only? It might fix bug 1415671 then...
> 
> If it wasn't Mac-only, I'd like to better understand what's causing the
> difference, as this is one of the primary UI pieces where we care about such
> minor stuff.

Yes mac only, as far as I can tell amongst all the noise.
https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=a5060cf8a8705577d58ba16067b3e5e842be395a&newProject=try&newRev=cdad026ab6d833ac224c6725709bfff2e14fef15

(I'll buy a drink for anyone who can find the 1px difference on the Windows comparisons, I can't)

(In reply to Andreea Pavel [:apavel] from comment #24)
> Backed out for failing e.g.
> testing/firefox-ui/tests/puppeteer/test_tabbar.py r=backout on a CLOSED TREE
> 
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=b97013a2147601ed4464172e3fa493c33f543d50
> 
> Failure logs:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=153111029&repo=autoland&lineNumber=22759
> 
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=153111640&repo=autoland&lineNumber=7621
> 
> Backout:
> https://hg.mozilla.org/integration/autoland/rev/
> 39f703c43a6b353d6525f6db4c1ad6324487b22c

So the test_tabbar.py failure isn't particularly interesting, just an assertion that needs updating (and I need to remember there're now other testsuites than mochitest-bc for chrome changes):
https://searchfox.org/mozilla-central/rev/22c55eb7b7e6494a8615a7af3b613ff899d2cdba/testing/firefox-ui/tests/puppeteer/test_tabbar.py#131

However the accessibility failures are much more valuable:
https://treeherder.mozilla.org/logviewer.html#?job_id=153111640&repo=autoland&lineNumber=7621
https://searchfox.org/mozilla-central/rev/22c55eb7b7e6494a8615a7af3b613ff899d2cdba/accessible/tests/mochitest/tree/test_tabbrowser.xul#136
I'm guessing this would have regressed the experience for screen reader etc users, since the close button would no longer have been annotated as a button.  role="button" is enough to fix it, so I'll update the patch with that and the assertion change shortly, but I won't request re-review until after the holidays.
(though I now wonder why the mentioned mute tab button is role="presentation" instead of button)

Merry Christmas everyone!

Comment 26

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

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

Comment 27

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936599 [details]
Bug 1422934 - Remove unused toolbarbutton-image binding.

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

Comment 28

[Dão Gottwald [:dao]]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #25)
> However the accessibility failures are much more valuable:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=153111640&repo=autoland&lineNumber=7621
> https://searchfox.org/mozilla-central/rev/
> 22c55eb7b7e6494a8615a7af3b613ff899d2cdba/accessible/tests/mochitest/tree/
> test_tabbrowser.xul#136
> I'm guessing this would have regressed the experience for screen reader etc
> users, since the close button would no longer have been annotated as a
> button.  role="button" is enough to fix it, so I'll update the patch with
> that and the assertion change shortly, but I won't request re-review until
> after the holidays.
> (though I now wonder why the mentioned mute tab button is
> role="presentation" instead of button)

Because there's a keyboard shortcut for muting the selected tab. Of course there's one for closing the selected tab too. Neither shortcuts work for background tabs though. The idea that screenreader users would walk through the a11y tree to find a particular background tab's close or mute button seems a bit weird to me, but a11y folks will probably disagree.

Comment 29

[:Gijs (out for now; he/him)]

(In reply to Dão Gottwald [::dao] from comment #28)
> Because there's a keyboard shortcut for muting the selected tab. Of course
> there's one for closing the selected tab too. Neither shortcuts work for
> background tabs though. The idea that screenreader users would walk through
> the a11y tree to find a particular background tab's close or mute button
> seems a bit weird to me, but a11y folks will probably disagree.

I'm not sure they would... it's quite possible this is just legacy stuff that's never been updated. Marco, is there a good reason for having the tab close button be represented as a button in the a11y tree, given there's a keyboard shortcut as well as menu entries for it, and presumably no real need for people who use the a11y tree to access it that way - similar to the tab mute button?

Note that if we accept this not being a button, it should probably get role=presentation.

Comment 30

[:Gijs (out for now; he/him)]

(fwiw, rs=me on adding role=button/presentation based on what Marco says, no need to re-request review - Merry Christmas! :-) )

Comment 31

[Marco Zehe (:MarcoZ)]

(In reply to :Gijs from comment #29)
> I'm not sure they would... it's quite possible this is just legacy stuff
> that's never been updated. Marco, is there a good reason for having the tab
> close button be represented as a button in the a11y tree, given there's a
> keyboard shortcut as well as menu entries for it, and presumably no real
> need for people who use the a11y tree to access it that way - similar to the
> tab mute button?

I've not used it myself, because it is only accessible via techniques like the NVDA object navigator. And yes, a keyboard shortcut is a much faster way to do the same thing.

> Note that if we accept this not being a button, it should probably get
> role=presentation.

Yes, it should.

Comment 32

[Dão Gottwald [:dao]]

Ian, care to update the patch? You might also want to address bug 1426826.

Comment 33

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

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

Comment 34

[Ian Moody [:Kwan] (UTC+1)]

Comment on attachment 8936599 [details]
Bug 1422934 - Remove unused toolbarbutton-image binding.

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

Comment 35

[Dão Gottwald [:dao]]

Comment on attachment 8936598 [details]
Bug 1422934 - Make the tab close button an image, and remove the extends from its binding.

https://reviewboard.mozilla.org/r/207350/#review217848

Comment 37

[Ian Moody [:Kwan] (UTC+1)]

Thanks for the review of the CSS Dão!

Since it's so close to merge day I'm going to wait until 60 to land this so it gets a little longer to bake on nightly, just in-case we do get some negative accessible feedback.  While there is Ctrl+W for the current tab, the buttons also allow closing other tabs (at least when there aren't too many), which won't be possible now.  No idea how often that'd actually be used though.
(though feel free to disagree and request landing it.  I don't know if we even have much accessible use on nightly so it might be pointless)

Comment 38

[Dão Gottwald [:dao]]

(In reply to Ian Moody [:Kwan] (UTC+0) from comment #37)
> While there is Ctrl+W for the current tab,
> the buttons also allow closing other tabs (at least when there aren't too
> many), which won't be possible now.  No idea how often that'd actually be
> used though.

It's not like this ever worked well even if people may have tried it. The button doesn't currently have an accessible label.

I think this is good to land now.

Comment 39

[:Gijs (out for now; he/him)]

(In reply to Dão Gottwald [::dao] from comment #38)
> (In reply to Ian Moody [:Kwan] (UTC+0) from comment #37)
> > While there is Ctrl+W for the current tab,
> > the buttons also allow closing other tabs (at least when there aren't too
> > many), which won't be possible now.  No idea how often that'd actually be
> > used though.
> 
> It's not like this ever worked well even if people may have tried it. The
> button doesn't currently have an accessible label.
> 
> I think this is good to land now.

FWIW, +1 on this. I think it's perfectly OK to land now.

Comment 40

[Ian Moody [:Kwan] (UTC+1)]

Okay, I've kicked off a try (with the previous backout-causing ally and functional-ui jobs) and if it comes back green I'll request checkin (after I've checked if it's auto-rebasable):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e1fb676bedc845b47d1132fdf6416e65447c1d6

(hmm, meant to clear the needinfo with comment 37)

Comment 41

[Ian Moody [:Kwan] (UTC+1)]

Try run in comment 40 in green except for bug 1427510 due to old tree, and a couple intermittents.

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6346aa459b3
Make the tab close button an image, and remove the extends from its binding. r=dao,Gijs
https://hg.mozilla.org/integration/autoland/rev/8339907be1cd
Remove unused toolbarbutton-image binding. r=Gijs

Comment 43

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a6346aa459b3
https://hg.mozilla.org/mozilla-central/rev/8339907be1cd