Brendan Dahl [:bdahl] Assignee
	Description • 6 years ago

The commandDispatcher attribute needs to be moved from XULDocument.webidl to Document.webidl so that it works for chrome HTML pages.

	Brendan Dahl [:bdahl] Assignee
	Comment 1 • 6 years ago

The "commandDispatcher" and "addBroadcastListenerFor" attributes are going
to be removed from content privilege XUL.

MozReview-Commit-ID: DL8qxToeNMm

	Brendan Dahl [:bdahl] Assignee
	Comment 2 • 6 years ago

The "commandDispatcher" and "addBroadcastListenerFor" attributes are going
to be removed from content privilege XUL.

MozReview-Commit-ID: DL8qxToeNMm

	Brendan Dahl [:bdahl] Assignee
	Comment 3 • 6 years ago

The "commandDispatcher" and "addBroadcastListenerFor" attributes are going
to be removed from content privilege XUL.

MozReview-Commit-ID: DL8qxToeNMm

	Brendan Dahl [:bdahl] 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

	Brendan Dahl [:bdahl] 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

	Boris Zbarsky [: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?

	Brendan Dahl [:bdahl] 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.

	Boris Zbarsky [:bzbarsky]
	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

	Boris Zbarsky [:bzbarsky]
	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

	Brendan Dahl [:bdahl] 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

	Brendan Dahl [:bdahl] 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

	Brendan Dahl [:bdahl] 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.

	Brendan Dahl [:bdahl] Assignee
	Comment 13 • 6 years ago

Comment on attachment 8998559 [details] [diff] [review]
Move command dispatcher from XULDocument to Document.

Carrying r+ forward.

	Pulsebot
	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

	Andreea Pavel [:apavel]
	Comment 15 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/fc0dcbca21b5
https://hg.mozilla.org/mozilla-central/rev/271ad78ecd80