Closed Bug 1445292 Opened 5 years ago Closed 5 years ago

Use Services.els in tabbox.xml

Categories

(Toolkit :: XUL Widgets, enhancement)

Product:
Toolkit
Component:
XUL Widgets
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: dao, Assigned: vivek3zero, Mentored)

Details

(Keywords: good-first-bug, Whiteboard: [lang=js])

Attachments

(1 file)

Bug 1445292 - Use Services.els in tabbox.xml
59 bytes, text/x-review-board-request
dao
: review+
Details 
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
Attachment #8959548 - Flags: review?(dao+bmo) → review-
Assignee: nobody → vivek3zero
Should be fixed now!
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 :)
Attachment #8959548 - Flags: review?(dao+bmo) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d44271e5973
Use Services.els in tabbox.xml r=dao
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
Flags: needinfo?(vivek3zero)
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.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(vivek3zero)
Resolution: --- → WONTFIX
There's always more bugs to fix :D
You need to log in before you can comment on or make changes to this bug.