Reland "Update popover post-toggle event naming and behavior"
This is a reland of commit 2e5ee120d7fc5df4b6c101e88d03a5f62a73d8b8
It seems that the event coalescing behavior changes cause a segfault
in the event dispatch code. This reland (compare to Patchset 1) just
includes the event renaming, and not the new behavior. I'll land
the new behavior in a separate CL to make it smaller if it gets
reverted again. This part should be safe.
Original change's description:
> Update popover post-toggle event naming and behavior
>
> This CL updates the post-toggle event in the following ways:
> 1. Rename the 'aftertoggle' event to 'toggle'.
> 2. Rename PopoverToggleEvent to ToggleEvent.
> 3. Rename the currentState attribute to oldState.
> 4. Add event coalescing behavior. If two transitions occur before the
> first 'toggle' event has been fired, cancel the first event and
> queue a replacement that has oldState === newState.
>
> These changes were driven by the corresponding changes to the spec PR:
> https://github.com/whatwg/html/pull/8717
>
> Bug: 1307772
> Change-Id: Iabc5a9093d7cef3bbd6e54e488d8e571c51ea568
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195120
> Auto-Submit: Mason Freed <masonf@chromium.org>
> Commit-Queue: Joey Arhar <jarhar@chromium.org>
> Reviewed-by: Joey Arhar <jarhar@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1098728}
Bug: 1307772
Change-Id: Iede18bbf05516cec4ee881f1afc663c45380c82e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205710
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099397}
diff --git a/html/semantics/popovers/idlharness.tentative.html b/html/semantics/popovers/idlharness.tentative.html
index d1a258b..1cab891 100644
--- a/html/semantics/popovers/idlharness.tentative.html
+++ b/html/semantics/popovers/idlharness.tentative.html
@@ -41,12 +41,12 @@
'document.getElementById("b3")',
],
BeforeToggleEvent: [
- 'new PopoverToggleEvent("beforetoggle")',
- 'new PopoverToggleEvent("beforetoggle", {currentState: "open"})',
- 'new PopoverToggleEvent("beforetoggle", {currentState: "open",newState: "open"})',
- 'new PopoverToggleEvent("aftertoggle")',
- 'new PopoverToggleEvent("aftertoggle", {currentState: "open"})',
- 'new PopoverToggleEvent("aftertoggle", {currentState: "open",newState: "open"})',
+ 'new ToggleEvent("beforetoggle")',
+ 'new ToggleEvent("beforetoggle", {oldState: "open"})',
+ 'new ToggleEvent("beforetoggle", {oldState: "open",newState: "open"})',
+ 'new ToggleEvent("toggle")',
+ 'new ToggleEvent("toggle", {oldState: "open"})',
+ 'new ToggleEvent("toggle", {oldState: "open",newState: "open"})',
],
});
}
diff --git a/html/semantics/popovers/popover-events.tentative.html b/html/semantics/popovers/popover-events.tentative.html
index b96a0f5..b299424 100644
--- a/html/semantics/popovers/popover-events.tentative.html
+++ b/html/semantics/popovers/popover-events.tentative.html
@@ -10,6 +10,13 @@
<div popover>Popover</div>
<script>
+function getPopoverAndSignal(t) {
+ const popover = document.querySelector('[popover]');
+ const controller = new AbortController();
+ const signal = controller.signal;
+ t.add_cleanup(() => controller.abort());
+ return {popover, signal};
+}
window.onload = () => {
for(const method of ["listener","attribute"]) {
promise_test(async t => {
@@ -22,52 +29,48 @@
function listener(e) {
if (e.type === "beforetoggle") {
if (e.newState === "open") {
- assert_equals(e.currentState,"closed",'The "beforetoggle" event should be fired before the popover is open');
+ ++showCount;
+ assert_equals(e.oldState,"closed",'The "beforetoggle" event should be fired before the popover is open');
assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the opening event fires.');
assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the opening event fires.');
- ++showCount;
} else {
+ ++hideCount;
assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"');
- assert_equals(e.currentState,"open",'The "beforetoggle" event should be fired before the popover is closed')
+ assert_equals(e.oldState,"open",'The "beforetoggle" event should be fired before the popover is closed')
assert_true(e.target.matches(':open'),'The popover should be in the :open state when the hiding event fires.');
assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the hiding event fires.');
- ++hideCount;
}
} else {
- assert_equals(e.type,"aftertoggle",'Popover events should be "beforetoggle" and "aftertoggle"')
+ assert_equals(e.type,"toggle",'Popover events should be "beforetoggle" and "toggle"')
if (e.newState === "open") {
- assert_equals(e.currentState,"open",'Aftertoggle should be fired after the popover is open');
+ ++afterShowCount;
if (document.body.contains(e.target)) {
assert_true(e.target.matches(':open'),'The popover should be in the :open state when the after opening event fires.');
assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the after opening event fires.');
}
- ++afterShowCount;
} else {
+ ++afterHideCount;
assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"');
- assert_equals(e.currentState,"closed",'Aftertoggle should be fired after the popover is closed');
assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the after hiding event fires.');
assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the after hiding event fires.');
- ++afterHideCount;
}
- e.preventDefault(); // "aftertoggle" should not be cancelable.
+ e.preventDefault(); // "toggle" should not be cancelable.
}
};
switch (method) {
case "listener":
- const controller = new AbortController();
- const signal = controller.signal;
- t.add_cleanup(() => controller.abort());
+ const {signal} = getPopoverAndSignal(t);
// These events bubble.
document.addEventListener('beforetoggle', listener, {signal});
- document.addEventListener('aftertoggle', listener, {signal});
+ document.addEventListener('toggle', listener, {signal});
break;
case "attribute":
assert_false(popover.hasAttribute('onbeforetoggle'));
t.add_cleanup(() => popover.removeAttribute('onbeforetoggle'));
popover.onbeforetoggle = listener;
- assert_false(popover.hasAttribute('onaftertoggle'));
- t.add_cleanup(() => popover.removeAttribute('onaftertoggle'));
- popover.onaftertoggle = listener;
+ assert_false(popover.hasAttribute('ontoggle'));
+ t.add_cleanup(() => popover.removeAttribute('ontoggle'));
+ popover.ontoggle = listener;
break;
default: assert_unreached();
}
@@ -82,7 +85,7 @@
assert_equals(0,afterShowCount);
assert_equals(0,afterHideCount);
await waitForRender();
- assert_equals(1,afterShowCount,'aftertoggle show is fired asynchronously');
+ assert_equals(1,afterShowCount,'toggle show is fired asynchronously');
assert_equals(0,afterHideCount);
assert_true(popover.matches(':open'));
popover.hidePopover();
@@ -93,7 +96,7 @@
assert_equals(0,afterHideCount);
await waitForRender();
assert_equals(1,afterShowCount);
- assert_equals(1,afterHideCount,'aftertoggle hide is fired asynchronously');
+ assert_equals(1,afterHideCount,'toggle hide is fired asynchronously');
// No additional events
await waitForRender();
await waitForRender();
@@ -106,10 +109,7 @@
}
promise_test(async t => {
- const popover = document.querySelector('[popover]');
- const controller = new AbortController();
- const signal = controller.signal;
- t.add_cleanup(() => controller.abort());
+ const {popover,signal} = getPopoverAndSignal(t);
let cancel = true;
popover.addEventListener('beforetoggle',(e) => {
if (e.newState !== "open")
@@ -128,10 +128,7 @@
}, 'The "beforetoggle" event is cancelable for the "opening" transition');
promise_test(async t => {
- const popover = document.querySelector('[popover]');
- const controller = new AbortController();
- const signal = controller.signal;
- t.add_cleanup(() => {controller.abort();});
+ const {popover,signal} = getPopoverAndSignal(t);
popover.addEventListener('beforetoggle',(e) => {
assert_not_equals(e.newState,"closed",'The "beforetoggle" event was fired for the closing transition');
}, {signal});
@@ -144,5 +141,74 @@
await waitForRender(); // Check for async events also
assert_false(popover.matches(':open'));
}, 'The "beforetoggle" event is not fired for element removal');
+
+ promise_test(async t => {
+ const {popover,signal} = getPopoverAndSignal(t);
+ let events;
+ function resetEvents() {
+ events = {
+ singleShow: false,
+ singleHide: false,
+ coalescedShow: false,
+ coalescedHide: false,
+ };
+ }
+ function setEvent(type) {
+ assert_equals(events[type],false,'event repeated');
+ events[type] = true;
+ }
+ function assertOnly(type,msg) {
+ Object.keys(events).forEach(val => {
+ assert_equals(events[val],val===type,`${msg} (${val})`);
+ });
+ }
+ popover.addEventListener('toggle',(e) => {
+ switch (e.newState) {
+ case "open":
+ switch (e.oldState) {
+ case "open": setEvent('coalescedShow'); break;
+ case "closed": setEvent('singleShow'); break;
+ default: assert_unreached();
+ }
+ break;
+ case "closed":
+ switch (e.oldState) {
+ case "closed": setEvent('coalescedHide'); break;
+ case "open": setEvent('singleHide'); break;
+ default: assert_unreached();
+ }
+ break;
+ default: assert_unreached();
+ }
+ }, {signal});
+
+ resetEvents();
+ assertOnly('none');
+ assert_false(popover.matches(':open'));
+ popover.showPopover();
+ await waitForRender();
+ assert_true(popover.matches(':open'));
+ assertOnly('singleShow','Single event should have been fired, which is a "show"');
+
+ resetEvents();
+ popover.hidePopover();
+ popover.showPopover(); // Immediate re-show
+ await waitForRender();
+ assert_true(popover.matches(':open'));
+ assertOnly('coalescedShow','Single coalesced event should have been fired, which is a "show"');
+
+ resetEvents();
+ popover.hidePopover();
+ await waitForRender();
+ assertOnly('singleHide','Single event should have been fired, which is a "hide"');
+ assert_false(popover.matches(':open'));
+
+ resetEvents();
+ popover.showPopover();
+ popover.hidePopover(); // Immediate re-hide
+ await waitForRender();
+ assertOnly('coalescedHide','Single coalesced event should have been fired, which is a "hide"');
+ assert_false(popover.matches(':open'));
+ }, 'The "toggle" event is coalesced');
};
</script>
diff --git a/html/semantics/popovers/toggleevent-interface.tentative.html b/html/semantics/popovers/toggleevent-interface.tentative.html
index 4d437b0..4570945 100644
--- a/html/semantics/popovers/toggleevent-interface.tentative.html
+++ b/html/semantics/popovers/toggleevent-interface.tentative.html
@@ -7,200 +7,200 @@
<script>
test(function() {
- var event = new PopoverToggleEvent("");
- assert_true(event instanceof window.PopoverToggleEvent);
-}, "the event is an instance of PopoverToggleEvent");
+ var event = new ToggleEvent("");
+ assert_true(event instanceof window.ToggleEvent);
+}, "the event is an instance of ToggleEvent");
test(function() {
- var event = new PopoverToggleEvent("");
+ var event = new ToggleEvent("");
assert_true(event instanceof window.Event);
}, "the event inherts from Event");
test(function() {
assert_throws_js(TypeError, function() {
- new PopoverToggleEvent();
+ new ToggleEvent();
}, 'First argument (type) is required, so was expecting a TypeError.');
}, 'Missing type argument');
test(function() {
- var event = new PopoverToggleEvent("test");
+ var event = new ToggleEvent("test");
assert_equals(event.type, "test");
}, "type argument is string");
test(function() {
- var event = new PopoverToggleEvent(null);
+ var event = new ToggleEvent(null);
assert_equals(event.type, "null");
}, "type argument is null");
test(function() {
- var event = new PopoverToggleEvent(undefined);
+ var event = new ToggleEvent(undefined);
assert_equals(event.type, "undefined");
}, "event type set to undefined");
test(function() {
- var event = new PopoverToggleEvent("test");
- assert_equals(event.currentState, "");
-}, "currentState has default value of empty string");
+ var event = new ToggleEvent("test");
+ assert_equals(event.oldState, "");
+}, "oldState has default value of empty string");
test(function() {
- var event = new PopoverToggleEvent("test");
- assert_readonly(event, "currentState", "readonly attribute value");
-}, "currentState is readonly");
+ var event = new ToggleEvent("test");
+ assert_readonly(event, "oldState", "readonly attribute value");
+}, "oldState is readonly");
test(function() {
- var event = new PopoverToggleEvent("test");
+ var event = new ToggleEvent("test");
assert_equals(event.newState, "");
}, "newState has default value of empty string");
test(function() {
- var event = new PopoverToggleEvent("test");
+ var event = new ToggleEvent("test");
assert_readonly(event, "newState", "readonly attribute value");
}, "newState is readonly");
test(function() {
- var event = new PopoverToggleEvent("test", null);
- assert_equals(event.currentState, "");
+ var event = new ToggleEvent("test", null);
+ assert_equals(event.oldState, "");
assert_equals(event.newState, "");
-}, "PopoverToggleEventInit argument is null");
+}, "ToggleEventInit argument is null");
test(function() {
- var event = new PopoverToggleEvent("test", undefined);
- assert_equals(event.currentState, "");
+ var event = new ToggleEvent("test", undefined);
+ assert_equals(event.oldState, "");
assert_equals(event.newState, "");
-}, "PopoverToggleEventInit argument is undefined");
+}, "ToggleEventInit argument is undefined");
test(function() {
- var event = new PopoverToggleEvent("test", {});
- assert_equals(event.currentState, "");
+ var event = new ToggleEvent("test", {});
+ assert_equals(event.oldState, "");
assert_equals(event.newState, "");
-}, "PopoverToggleEventInit argument is empty dictionary");
+}, "ToggleEventInit argument is empty dictionary");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: "sample"});
- assert_equals(event.currentState, "sample");
-}, "currentState set to 'sample'");
+ var event = new ToggleEvent("test", {oldState: "sample"});
+ assert_equals(event.oldState, "sample");
+}, "oldState set to 'sample'");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: undefined});
- assert_equals(event.currentState, "");
-}, "currentState set to undefined");
+ var event = new ToggleEvent("test", {oldState: undefined});
+ assert_equals(event.oldState, "");
+}, "oldState set to undefined");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: null});
- assert_equals(event.currentState, "null");
-}, "currentState set to null");
+ var event = new ToggleEvent("test", {oldState: null});
+ assert_equals(event.oldState, "null");
+}, "oldState set to null");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: false});
- assert_equals(event.currentState, "false");
-}, "currentState set to false");
+ var event = new ToggleEvent("test", {oldState: false});
+ assert_equals(event.oldState, "false");
+}, "oldState set to false");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: true});
- assert_equals(event.currentState, "true");
-}, "currentState set to true");
+ var event = new ToggleEvent("test", {oldState: true});
+ assert_equals(event.oldState, "true");
+}, "oldState set to true");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: 0.5});
- assert_equals(event.currentState, "0.5");
-}, "currentState set to a number");
+ var event = new ToggleEvent("test", {oldState: 0.5});
+ assert_equals(event.oldState, "0.5");
+}, "oldState set to a number");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: []});
- assert_equals(event.currentState, "");
-}, "currentState set to []");
+ var event = new ToggleEvent("test", {oldState: []});
+ assert_equals(event.oldState, "");
+}, "oldState set to []");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: [1, 2, 3]});
- assert_equals(event.currentState, "1,2,3");
-}, "currentState set to [1, 2, 3]");
+ var event = new ToggleEvent("test", {oldState: [1, 2, 3]});
+ assert_equals(event.oldState, "1,2,3");
+}, "oldState set to [1, 2, 3]");
test(function() {
- var event = new PopoverToggleEvent("test", {currentState: {sample: 0.5}});
- assert_equals(event.currentState, "[object Object]");
-}, "currentState set to an object");
+ var event = new ToggleEvent("test", {oldState: {sample: 0.5}});
+ assert_equals(event.oldState, "[object Object]");
+}, "oldState set to an object");
test(function() {
- var event = new PopoverToggleEvent("test",
- {currentState: {valueOf: function () { return 'sample'; }}});
- assert_equals(event.currentState, "[object Object]");
-}, "currentState set to an object with a valueOf function");
+ var event = new ToggleEvent("test",
+ {oldState: {valueOf: function () { return 'sample'; }}});
+ assert_equals(event.oldState, "[object Object]");
+}, "oldState set to an object with a valueOf function");
test(function() {
- var eventInit = {currentState: "sample",newState: "sample2"};
- var event = new PopoverToggleEvent("test", eventInit);
- assert_equals(event.currentState, "sample");
+ var eventInit = {oldState: "sample",newState: "sample2"};
+ var event = new ToggleEvent("test", eventInit);
+ assert_equals(event.oldState, "sample");
assert_equals(event.newState, "sample2");
-}, "PopoverToggleEventInit properties set value");
+}, "ToggleEventInit properties set value");
test(function() {
- var eventInit = {currentState: "open",newState: "closed"};
- var event = new PopoverToggleEvent("beforetoggle", eventInit);
- assert_equals(event.currentState, "open");
+ var eventInit = {oldState: "open",newState: "closed"};
+ var event = new ToggleEvent("beforetoggle", eventInit);
+ assert_equals(event.oldState, "open");
assert_equals(event.newState, "closed");
-}, "PopoverToggleEventInit properties set value 2");
+}, "ToggleEventInit properties set value 2");
test(function() {
- var eventInit = {currentState: "closed",newState: "open"};
- var event = new PopoverToggleEvent("aftertoggle", eventInit);
- assert_equals(event.currentState, "closed");
+ var eventInit = {oldState: "closed",newState: "open"};
+ var event = new ToggleEvent("toggle", eventInit);
+ assert_equals(event.oldState, "closed");
assert_equals(event.newState, "open");
-}, "PopoverToggleEventInit properties set value 3");
+}, "ToggleEventInit properties set value 3");
test(function() {
- var eventInit = {currentState: "open",newState: "open"};
- var event = new PopoverToggleEvent("beforetoggle", eventInit);
- assert_equals(event.currentState, "open");
+ var eventInit = {oldState: "open",newState: "open"};
+ var event = new ToggleEvent("beforetoggle", eventInit);
+ assert_equals(event.oldState, "open");
assert_equals(event.newState, "open");
-}, "PopoverToggleEventInit properties set value 4");
+}, "ToggleEventInit properties set value 4");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: "sample"});
+ var event = new ToggleEvent("test", {newState: "sample"});
assert_equals(event.newState, "sample");
}, "newState set to 'sample'");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: undefined});
+ var event = new ToggleEvent("test", {newState: undefined});
assert_equals(event.newState, "");
}, "newState set to undefined");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: null});
+ var event = new ToggleEvent("test", {newState: null});
assert_equals(event.newState, "null");
}, "newState set to null");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: false});
+ var event = new ToggleEvent("test", {newState: false});
assert_equals(event.newState, "false");
}, "newState set to false");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: true});
+ var event = new ToggleEvent("test", {newState: true});
assert_equals(event.newState, "true");
}, "newState set to true");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: 0.5});
+ var event = new ToggleEvent("test", {newState: 0.5});
assert_equals(event.newState, "0.5");
}, "newState set to a number");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: []});
+ var event = new ToggleEvent("test", {newState: []});
assert_equals(event.newState, "");
}, "newState set to []");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: [1, 2, 3]});
+ var event = new ToggleEvent("test", {newState: [1, 2, 3]});
assert_equals(event.newState, "1,2,3");
}, "newState set to [1, 2, 3]");
test(function() {
- var event = new PopoverToggleEvent("test", {newState: {sample: 0.5}});
+ var event = new ToggleEvent("test", {newState: {sample: 0.5}});
assert_equals(event.newState, "[object Object]");
}, "newState set to an object");
test(function() {
- var event = new PopoverToggleEvent("test",
+ var event = new ToggleEvent("test",
{newState: {valueOf: function () { return 'sample'; }}});
assert_equals(event.newState, "[object Object]");
}, "newState set to an object with a valueOf function");
diff --git a/interfaces/popover.tentative.idl b/interfaces/popover.tentative.idl
index bf23c76..4e8c85f 100644
--- a/interfaces/popover.tentative.idl
+++ b/interfaces/popover.tentative.idl
@@ -13,13 +13,13 @@
HTMLInputElement includes PopoverTargetElement;
HTMLButtonElement includes PopoverTargetElement;
-interface PopoverToggleEvent : Event {
- constructor(DOMString type, optional PopoverToggleEventInit eventInitDict = {});
- readonly attribute DOMString currentState;
+interface ToggleEvent : Event {
+ constructor(DOMString type, optional ToggleEventInit eventInitDict = {});
+ readonly attribute DOMString oldState;
readonly attribute DOMString newState;
};
-dictionary PopoverToggleEventInit : EventInit {
- DOMString currentState = "";
+dictionary ToggleEventInit : EventInit {
+ DOMString oldState = "";
DOMString newState = "";
};
