Skip to content

Commit

Permalink
Only reflect valid popup values
Browse files Browse the repository at this point in the history
This was resolved in [1] - only valid values for the `popup` attribute
should be reflected, so that feature detection can be used on the values.

[1] openui/open-ui#491 (comment)

Bug: 1307772
Change-Id: I932a28bd48e336cf01253a971804dc00dcd099fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3639122
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001641}
NOKEYCHECK=True
GitOrigin-RevId: 21e6efd79ab1fecf1b95c3fb35a8133d801c1396
  • Loading branch information
mfreed7 authored and copybara-github committed May 10, 2022
1 parent d724a13 commit a420056
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
2 changes: 1 addition & 1 deletion blink/renderer/core/dom/element.idl
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ dictionary IsVisibleOptions {
// The Popup API
[MeasureAs=ElementShowPopup,RuntimeEnabled=HTMLPopupAttribute,RaisesException] void showPopup();
[MeasureAs=ElementHidePopup,RuntimeEnabled=HTMLPopupAttribute,RaisesException] void hidePopup();
[Unscopable,CEReactions,RuntimeEnabled=HTMLPopupAttribute,Reflect] attribute DOMString popup;
[Unscopable,CEReactions,RuntimeEnabled=HTMLPopupAttribute,Reflect,ReflectOnly=("popup","hint","async"),ReflectMissing="",ReflectInvalid=""] attribute DOMString popup;
[CEReactions,RuntimeEnabled=HTMLPopupAttribute,Reflect] attribute boolean defaultOpen;

// Experimental accessibility API
Expand Down
6 changes: 6 additions & 0 deletions blink/renderer/core/html/keywords.json5
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@
"eager",
"auto",

// popup attribute (experimental)
// https://github.com/openui/open-ui/blob/main/research/src/pages/popup/popup.research.explainer.mdx
"popup",
"hint",
"async",

// referrerpolicy attribute
// https://w3c.github.io/webappsec-referrer-policy/#referrer-policies
"no-referrer",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,27 @@
popup.setAttribute('popup','hint');
assert_equals(popup.popup,'hint');
popup.setAttribute('popup','invalid');
assert_equals(popup.popup,'invalid');
assert_equals(popup.popup,'','Invalid values should reflect as empty string');
popup.setAttribute('popup','HiNt');
assert_equals(popup.popup,'HiNt');
assert_equals(popup.popup,'hint','Case is normalized');
popup.removeAttribute('popup');
assert_equals(popup.popup,'');
assert_equals(popup.popup,'','No value should reflect as empty string');
popup.popup='hint';
assert_equals(popup.getAttribute('popup'),'hint');
popup.popup='popup';
assert_equals(popup.getAttribute('popup'),'popup');
popup.popup='invalid';
assert_equals(popup.getAttribute('popup'),'invalid');
assert_equals(popup.getAttribute('popup'),'invalid','IDL setter allows any value');
assert_equals(popup.popup,'','but IDL getter does not re-reflect invalid values');
popup.popup='';
assert_equals(popup.getAttribute('popup'),'');
assert_equals(popup.getAttribute('popup'),'','Empty should map to empty');
assert_equals(popup.popup,'','Empty should map to empty in IDL');
popup.popup=null;
assert_equals(popup.getAttribute('popup'),'null','Attribute for "null" should still get set');
assert_equals(popup.popup,'','Null should map to empty in IDL');
popup.popup=undefined;
assert_equals(popup.getAttribute('popup'),'undefined','Attribute for "undefined" should still get set');
assert_equals(popup.popup,'','undefined should map to empty in IDL');
},'IDL attribute reflection');

test(() => {
Expand All @@ -110,6 +118,8 @@
assertIsFunctionalPopup(popup);
popup.popup = 'PoPuP';
assertIsFunctionalPopup(popup);
popup.popup = 'invalid';
assertNotAPopup(popup);
},'Popup attribute value should be case insensitive');

test(() => {
Expand Down

0 comments on commit a420056

Please sign in to comment.