1445292 - Use Services.els in tabbox.xml

Status: RESOLVED WONTFIX

(Toolkit :: UI Widgets, enhancement)

Description

[Dão Gottwald [:dao]]

Instead of the els variable, we should just use Services.els here:

https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/toolkit/content/widgets/tabbox.xml#193-194

https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/toolkit/content/widgets/tabbox.xml#210

https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/toolkit/content/widgets/tabbox.xml#216

Comment 1

[Vivek]

Attachment: Bug 1445292 - Use Services.els in tabbox.xml

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

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8959548 [details]
Bug 1445292 - Use Services.els in tabbox.xml

https://reviewboard.mozilla.org/r/228354/#review234244

::: toolkit/content/widgets/tabbox.xml:192
(Diff revision 1)
>            <![CDATA[
>              if (val != this._eventNode) {
>                const nsIEventListenerService =
>                  Ci.nsIEventListenerService;
>                let els = Cc["@mozilla.org/eventlistenerservice;1"]
>                            .getService(nsIEventListenerService);

Please remove const nsIEventListenerService and let els as they are unused now.

::: toolkit/content/widgets/tabbox.xml:209
(Diff revision 1)
>            case "parent": this._eventNode = this.parentNode; break;
>            case "window": this._eventNode = window; break;
>            case "document": this._eventNode = document; break;
>          }
>          let els = Cc["@mozilla.org/eventlistenerservice;1"]
>                      .getService(Ci.nsIEventListenerService);

same here

::: toolkit/content/widgets/tabbox.xml:215
(Diff revision 1)
> -        els.addSystemEventListener(this._eventNode, "keydown", this, false);
> +        Services.els.addSystemEventListener(this._eventNode, "keydown", this, false);
>        </constructor>
>  
>        <destructor>
>          let els = Cc["@mozilla.org/eventlistenerservice;1"]
>                      .getService(Ci.nsIEventListenerService);

same here

Comment 3

[Vivek]

Comment on attachment 8959548 [details]
Bug 1445292 - Use Services.els in tabbox.xml

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

Comment 4

[Vivek]

Should be fixed now!

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8959548 [details]
Bug 1445292 - Use Services.els in tabbox.xml

https://reviewboard.mozilla.org/r/228354/#review234252

Yep, this looks better, thanks :)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d44271e5973
Use Services.els in tabbox.xml r=dao

Comment 7

[Raul Gurzau (:RaulG)]

Backed out changeset 8d44271e5973 (bug 1445292) for c3 failures on toolkit/content/tests/chrome/test_tabbox.xul on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/a930e1f75ba58c222dc578b1e9ff3a853290ae74

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=168543345&repo=autoland&lineNumber=5358

[task 2018-03-16T16:11:05.600Z] 16:11:05     INFO - TEST-START | toolkit/content/tests/chrome/test_tabbox.xul
[task 2018-03-16T16:11:05.679Z] 16:11:05     INFO - TEST-INFO | started process screentopng
[task 2018-03-16T16:11:06.135Z] 16:11:06     INFO - TEST-INFO | screentopng: exit 0
[task 2018-03-16T16:11:06.137Z] 16:11:06     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_tabbox.xul | uncaught exception - ReferenceError: Services is not defined at tabbox_XBL_Constructor@chrome://global/content/bindings/tabbox.xml:205:9
[task 2018-03-16T16:11:06.137Z] 16:11:06     INFO - 
[task 2018-03-16T16:11:06.138Z] 16:11:06     INFO - simpletestOnerror@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1645:11
[task 2018-03-16T16:11:06.139Z] 16:11:06     INFO - OnErrorEventHandlerNonNull*@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1625:1
[task 2018-03-16T16:11:06.140Z] 16:11:06     INFO - GECKO(4550) | JavaScript error: chrome://global/content/bindings/tabbox.xml, line 205: ReferenceError: Services is not defined
[task 2018-03-16T16:11:06.142Z] 16:11:06     INFO - Not taking screenshot here: see the one that was previously logged

Comment 8

[Raul Gurzau (:RaulG)]

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

Comment 9

[Dão Gottwald [:dao]]

Ah, test_tabbox.xul doesn't import Services.jsm. Probably not worth fixing this then, until we migrate the tabbox binding to a JS class... Sorry, Vivek.

Comment 10

[Vivek]

There's always more bugs to fix :D