Closed
Bug 1481286
Opened 6 years ago
Closed 6 years ago
Move command dispatcher from XULDocument to Document
Categories
(Core :: DOM: Core & HTML, enhancement, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: bdahl, Assigned: bdahl)
References
Details
Attachments
(2 files, 5 obsolete files)
7.08 KB,
patch
|
bdahl
:
review+
|
Details | Diff | Splinter Review |
17.34 KB,
patch
|
bdahl
:
review+
|
Details | Diff | Splinter Review |
The commandDispatcher attribute needs to be moved from XULDocument.webidl to Document.webidl so that it works for chrome HTML pages.
Priority: -- → P3
Assignee | ||
Comment 1•6 years ago
|
||
The "commandDispatcher" and "addBroadcastListenerFor" attributes are going
to be removed from content privilege XUL.
MozReview-Commit-ID: DL8qxToeNMm
Assignee | ||
Updated•6 years ago
|
Attachment #8998266 -
Attachment is obsolete: true
Assignee | ||
Comment 2•6 years ago
|
||
The "commandDispatcher" and "addBroadcastListenerFor" attributes are going
to be removed from content privilege XUL.
MozReview-Commit-ID: DL8qxToeNMm
Assignee | ||
Updated•6 years ago
|
Attachment #8998275 -
Attachment is obsolete: true
Assignee | ||
Comment 3•6 years ago
|
||
The "commandDispatcher" and "addBroadcastListenerFor" attributes are going
to be removed from content privilege XUL.
MozReview-Commit-ID: DL8qxToeNMm
Attachment #8998298 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•6 years ago
|
||
Create a "commandset" custom element that performs the job of adding and
removing command updaters that XULDocument use to do. Previously, the
"commandupdater" attribute was allowed on any element, but in tree it is
only every used on "commandset" elements.
MozReview-Commit-ID: HUXMG9kx4ft
Attachment #8998299 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 5•6 years ago
|
||
Allows non-XUL chrome privilege documents to also use the command
dispatcher. The command dispatcher is created lazily since it will not
always be used.
MozReview-Commit-ID: HUXMG9kx4ft
Attachment #8998300 -
Flags: review?(bzbarsky)
Comment 6•6 years ago
|
||
Comment on attachment 8998298 [details] [diff] [review]
Update test to reflect removal of content XUL attributes.
This should be merged with the relevant removal changesets, right?
Attachment #8998298 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #6)
> Comment on attachment 8998298 [details] [diff] [review]
> Update test to reflect removal of content XUL attributes.
>
> This should be merged with the relevant removal changesets, right?
I got a little ahead of myself. I'll remove the test for 'addBroadcastListenerFor' in another bug and squash this commit into the third commit.
Comment 8•6 years ago
|
||
Comment on attachment 8998299 [details] [diff] [review]
Use a custom element to add/remove command updater.
>+ if (this.getAttribute("commandupdater")) {
This doesn't match the old code (which compared the value to "true"). Do we want to preserve the old behavior, or just test for the attribute's presence? If the latter, use hasAttribute() instead. This applies both places we do this check.
>+ if (events === "") {
If the events attr is not set, you will have null === "" here (modulo the XULElement weirdness around getAttribute, but I hope that will die sometime), which will test false. This should use ==, not ===, with a comment explaining why. Or just:
events = events || "*";
>+ if (targets === "") {
Same thing.
r=me
Attachment #8998299 -
Flags: review?(bzbarsky) → review+
Comment 9•6 years ago
|
||
Comment on attachment 8998300 [details] [diff] [review]
Move command dispatcher from XULDocument to Document.
>+ readonly attribute XULCommandDispatcher? commandDispatcher;
Please document when this returns null.
r=me
Attachment #8998300 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 10•6 years ago
|
||
Create a "commandset" custom element that performs the job of adding and
removing command updaters that XULDocument use to do. Previously, the
"commandupdater" attribute was allowed on any element, but in tree it is
only every used on "commandset" elements.
MozReview-Commit-ID: HUXMG9kx4ft
Attachment #8998558 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•6 years ago
|
Attachment #8998299 -
Attachment is obsolete: true
Assignee | ||
Comment 11•6 years ago
|
||
Allows non-XUL chrome privilege documents to also use the command
dispatcher. The command dispatcher is created lazily since it will not
always be used.
Update test to reflect removal of the XUL attribute "commandDispatcher"
from content privilege XUL.
MozReview-Commit-ID: HUXMG9kx4ft
Attachment #8998559 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•6 years ago
|
Attachment #8998298 -
Attachment is obsolete: true
Attachment #8998300 -
Attachment is obsolete: true
Assignee | ||
Comment 12•6 years ago
|
||
Comment on attachment 8998558 [details] [diff] [review]
Use a custom element to add/remove command updater.
Carrying r+ forward.
Attachment #8998558 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 13•6 years ago
|
||
Comment on attachment 8998559 [details] [diff] [review]
Move command dispatcher from XULDocument to Document.
Carrying r+ forward.
Attachment #8998559 -
Flags: review?(bzbarsky) → review+
Comment 14•6 years ago
|
||
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc0dcbca21b5
Use a custom element to add/remove command updater. r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/271ad78ecd80
Move command dispatcher from XULDocument to Document. r=bz
Comment 15•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fc0dcbca21b5
https://hg.mozilla.org/mozilla-central/rev/271ad78ecd80
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•