Skip to content

Commit

Permalink
Remove popup=hint functionality
Browse files Browse the repository at this point in the history
Per the [1] resolution, we're going to wait to spec/implement
popup=hint, until we have a chance to resolve some of the blocking
issues. This CL removes all functionality for popup=hint. I thought
about adding another flag to gate this functionality, but the logic
is already complex, and I didn't want to complicate it further.
When the time comes to put it back, this CL can be reverted.

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

Bug: 1307772
Change-Id: Ic9122d7e362eb1c57ef7ea8b6e080a866fca5724
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3957293
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1063078}
  • Loading branch information
Mason Freed authored and chromium-wpt-export-bot committed Oct 25, 2022
1 parent c99c08d commit ad8a1a6
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 267 deletions.
12 changes: 5 additions & 7 deletions html/semantics/popups/popup-appearance-ref.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@

<style>
.fake-pop-up {top: 100px; bottom: auto;}
#blank {left: -400px;}
#auto {left: -200px;}
#hint {left: 0;}
#manual {left: 200px;}
#invalid {left: 400px;}
#blank {left: -300px;}
#auto {left: -100px;}
#manual {left: 100px;}
#invalid {left: 300px;}
</style>

<p>There should be five pop-ups with similar appearance.</p>
<p>There should be four pop-ups with similar appearance.</p>
<div class="fake-pop-up" id=blank>Blank</div>
<div class="fake-pop-up" id=auto>Auto</div>
<div class="fake-pop-up" id=hint>Hint</div>
<div class="fake-pop-up" id=manual>Manual</div>
<div class="fake-pop-up" id=invalid>Invalid</div>
12 changes: 5 additions & 7 deletions html/semantics/popups/popup-appearance.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,16 @@

<style>
[popup] {top: 100px; bottom: auto;}
[popup=""] {left: -400px}
[popup=auto] {left: -200px; }
[popup=hint] {left: 0; }
[popup=manual] {left: 200px; }
[popup=invalid] {left: 400px; }
[popup=""] {left: -300px}
[popup=auto] {left: -100px; }
[popup=manual] {left: 100px; }
[popup=invalid] {left: 300px; }
</style>

<p>There should be five pop-ups with similar appearance.</p>
<p>There should be four pop-ups with similar appearance.</p>
<div popup>Blank
<div popup=auto>Auto</div>
</div>
<div popup=hint>Hint</div>
<div popup=manual>Manual</div>
<!-- This ensures unsupported popup values are treated as popup=manual -->
<div popup=invalid>Invalid</div>
Expand Down
36 changes: 14 additions & 22 deletions html/semantics/popups/popup-attribute-basic.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
<div popup id=boolean>Pop up</div>
<div popup="">Pop up</div>
<div popup=auto>Pop up</div>
<div popup=hint>Pop up</div>
<div popup=manual>Pop up</div>
<article popup>Different element type</article>
<header popup>Different element type</header>
Expand Down Expand Up @@ -116,20 +115,18 @@
// Getting the `popUp` IDL value only retrieves valid values.
const popUp = createPopUp(t);
assert_equals(popUp.popUp,'auto');
popUp.setAttribute('popup','hint');
assert_equals(popUp.popUp,'hint');
popUp.setAttribute('popup','HiNt');
assert_equals(popUp.popUp,'hint','Case is normalized in IDL');
assert_equals(popUp.getAttribute('popup'),'HiNt','Case is *not* normalized/changed in the content attribute');
popUp.popUp='hInT';
assert_equals(popUp.popUp,'hint','Case is normalized in IDL');
assert_equals(popUp.getAttribute('popup'),'hInT','Value set from IDL is propagated exactly to the content attribute');
popUp.setAttribute('popup','auto');
assert_equals(popUp.popUp,'auto');
popUp.setAttribute('popup','AuTo');
assert_equals(popUp.popUp,'auto','Case is normalized in IDL');
assert_equals(popUp.getAttribute('popup'),'AuTo','Case is *not* normalized/changed in the content attribute');
popUp.popUp='aUtO';
assert_equals(popUp.popUp,'auto','Case is normalized in IDL');
assert_equals(popUp.getAttribute('popup'),'aUtO','Value set from IDL is propagated exactly to the content attribute');
popUp.setAttribute('popup','invalid');
assert_equals(popUp.popUp,'manual','Invalid values should reflect as "manual"');
popUp.removeAttribute('popup');
assert_equals(popUp.popUp,null,'No value should reflect as null');
popUp.popUp='hint';
assert_equals(popUp.getAttribute('popup'),'hint');
popUp.popUp='auto';
assert_equals(popUp.getAttribute('popup'),'auto');
popUp.popUp='';
Expand Down Expand Up @@ -176,11 +173,11 @@
test((t) => {
const popUp = createPopUp(t);
assertIsFunctionalPopUp(popUp);
popUp.setAttribute('popup','hint'); // Change pop-up type
popUp.setAttribute('popup','manual'); // Change pop-up type
assertIsFunctionalPopUp(popUp);
popUp.setAttribute('popup','invalid'); // Change pop-up type to something invalid
assertIsFunctionalPopUp(popUp);
popUp.popUp = 'hint'; // Change pop-up type via IDL
popUp.popUp = 'manual'; // Change pop-up type via IDL
assertIsFunctionalPopUp(popUp);
popUp.popUp = 'invalid'; // Make invalid via IDL (treated as "manual")
assertIsFunctionalPopUp(popUp);
Expand All @@ -190,11 +187,7 @@
const popUp = createPopUp(t);
popUp.showPopUp();
assert_true(popUp.matches(':open'));
popUp.setAttribute('popup','hint'); // Change pop-up type
assert_false(popUp.matches(':open'));
popUp.showPopUp();
assert_true(popUp.matches(':open'));
popUp.setAttribute('popup','manual');
popUp.setAttribute('popup','manual'); // Change pop-up type
assert_false(popUp.matches(':open'));
popUp.showPopUp();
assert_true(popUp.matches(':open'));
Expand All @@ -217,7 +210,8 @@
}
}

["auto","hint","manual"].forEach(type => {
const validTypes = ["auto","manual"];
validTypes.forEach(type => {
test((t) => {
const popUp = createPopUp(t);
popUp.setAttribute('popup',type);
Expand All @@ -241,7 +235,6 @@
}
});

const validTypes = ["auto","hint","manual"];
function interpretedType(typeString,method) {
if (validTypes.includes(typeString))
return typeString;
Expand Down Expand Up @@ -308,8 +301,7 @@
popUp.hidePopUp();
break;
case 'auto':
case 'hint':
assert_false(popUp.matches(':open'),'A popup=auto/hint should light-dismiss');
assert_false(popUp.matches(':open'),'A popup=auto should light-dismiss');
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<p>Test for [popup]::backdrop presence and stacking order. The test passes
if there are 3 stacked boxes, with the brightest green on top.</p>
<div popup id=bottom>Bottom
<div popup=hint id=middle>Middle
<div popup id=middle>Middle
<div popup=manual id=top>Top</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<p>Test for [popup]::backdrop presence and stacking order. The test passes
if there are 3 stacked boxes, with the brightest green on top.</p>
<div popup id=bottom>Bottom
<div popup=hint id=middle>Middle
<div popup id=middle>Middle
<div popup=manual id=top>Top</div>
</div>
</div>
Expand Down
20 changes: 0 additions & 20 deletions html/semantics/popups/popup-defaultopen-hints.tentative.html

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
...unsupportedTypes,
];
window.addEventListener('load', () => {
["auto","hint","manual"].forEach(type => {
["auto","manual"].forEach(type => {
invokers.forEach(testcase => {
let t_set = [1], s_set = [1], h_set = [1];
if (testcase.supported) {
Expand Down
47 changes: 0 additions & 47 deletions html/semantics/popups/popup-light-dismiss.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -458,53 +458,6 @@
},'Ensure circular/convoluted ancestral relationships are functional, with a direct showPopUp()');
</script>


<div popup id=p10>Popup</div>
<div popup=hint id=p11>Hint</div>
<div popup=manual id=p12>Manual</div>
<style>
#p10 {top:100px;}
#p11 {top:200px;}
#p12 {top:300px;}
</style>
<script>
promise_test(async () => {
const auto = document.querySelector('#p10');
const hint = document.querySelector('#p11');
const manual = document.querySelector('#p12');
// All three can be open at once, if shown in this order:
auto.showPopUp();
hint.showPopUp();
manual.showPopUp();
assert_true(auto.matches(':open'));
assert_true(hint.matches(':open'));
assert_true(manual.matches(':open'));
// Clicking the hint will close the auto, but not the manual.
await clickOn(hint);
assert_false(auto.matches(':open'),'auto should be hidden');
assert_true(hint.matches(':open'),'hint should stay open');
assert_true(manual.matches(':open'),'manual does not light dismiss');
// Clicking outside should close the hint, but not the manual:
await clickOn(outside);
assert_false(auto.matches(':open'));
assert_false(hint.matches(':open'),'hint should close');
assert_true(manual.matches(':open'),'manual does not light dismiss');
manual.hidePopUp();
assert_false(manual.matches(':open'));
auto.showPopUp();
hint.showPopUp();
assert_true(auto.matches(':open'));
assert_true(hint.matches(':open'));
// Clicking on the auto should close the hint:
await clickOn(auto);
assert_true(auto.matches(':open'),'auto should stay open');
assert_false(hint.matches(':open'),'hint should light dismiss');
auto.hidePopUp();
assert_false(auto.matches(':open'));
},'Light dismiss of mixed popup types');
</script>


<div popup id=p13>Pop-up 1
<div popup id=p14>Pop-up 2
<div popup id=p15>Pop-up 3</div>
Expand Down
12 changes: 5 additions & 7 deletions html/semantics/popups/popup-manual-crash.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@

<style>
[popup] {top: 100px; bottom: auto;}
[popup=""] {left: -300px}
[popup=auto] {left: -100px; }
[popup=hint] {left: 100px; }
[popup=manual] {left: 300px; }
[popup=""] {left: -200px}
[popup=auto] {left: 0; }
[popup=manual] {left: 200px; }
</style>

<p>This test passes if it does not crash.</p>
<div popup>Blank
<div popup=auto>Auto</div>
<div popup>Auto1
<div popup=auto>Auto2</div>
</div>
<div popup=hint>Hint</div>
<div popup=manual>Manual</div>
<script>
document.querySelectorAll('[popup]').forEach(p => p.showPopUp());
Expand Down
Loading

0 comments on commit ad8a1a6

Please sign in to comment.