Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beforematch event and hidden=until-found #511

Closed
1 task
josepharhar opened this issue May 1, 2020 · 18 comments
Closed
1 task

Beforematch event and hidden=until-found #511

josepharhar opened this issue May 1, 2020 · 18 comments
Assignees
Labels
Resolution: satisfied The TAG is satisfied with this design Review type: CG early review An early review of general direction from a Community Group Venue: CSS WG Venue: WHATWG Venue: WICG

Comments

@josepharhar
Copy link

josepharhar commented May 1, 2020

Hello TAG!

I'm requesting a TAG review of the beforematch event.

The beforematch event allows developers to display content to the user in response to the following actions:

  • find-in-page (ctrl+f)
  • element fragment navigation (example.com/#foo)
  • scroll-to-text navigation (example.com/#:~:text=foo)

The event is fired at render timing before these actions scroll the page. The event is fired on the element which contains the text, or in the case of element fragment navigation, whichever element has the target id.

Further details:

We'd prefer the TAG provide feedback as (please delete all but the desired option):

🐛 open issues in our GitHub repo for each point of feedback

@josepharhar josepharhar added Progress: untriaged Review type: CG early review An early review of general direction from a Community Group labels May 1, 2020
@kenchris kenchris self-assigned this May 1, 2020
@alice alice self-assigned this May 13, 2020
@plinss plinss added this to the 2020-05-21-f2f milestone May 13, 2020
@dbaron
Copy link
Member

dbaron commented May 13, 2020

We discussed this a bit in #306 in the past.

@alice
Copy link

alice commented May 27, 2020

Thanks for bringing this review to us. We are looking at this at our virtual face-to-face.

This seems intrinsically linked to the content-visibility: hidden-matchable state described in the Display Locking explainer, so it would be helpful to have a bit more discussion on how that state came to be - specifically, why content-visibility: auto isn't sufficient for the "Deep links and searchability into pages with hidden content" case, which this proposal seems primarily concerned with.

I struggled to fully understand the use case, even having read through the earlier discussion on #306, this explainer, and the Display Locking explainer mentioned above. Would it be possible to have a clearer description of exactly what problem this is solving?

One other note: "beforematch is a useful signal to the page which allows custom styling of the matched element, which is now only possible with approximations from scroll positions and intersection observations" - this sounds like a use case that would be better served by a CSS pseudo-selector.

@atanassov
Copy link

Also from the same virtual face-to-face review. My main concern with the proposal is somewhat captured with issue 150. The beforematch event opens the door for re-entrant code introducing easy to achieve circularity. The beforematch event fires >> the event handler removes the target DOM element >> another event handler handles the remove and adds it back to DOM >> etc.

I appreciate that you're upfront listing the issues in your explainer however I didn't see good mitigation besides some ideas around scrolling specific logic. Can you provide more details as to how do you intend to stabilize this?

@vmpstr
Copy link

vmpstr commented May 28, 2020

(Just some logistics changes; we restructured the repo a little bit so the links have changed:

  • beforematch explainer is here
  • Security and Privacy self-review is here

All of these should be discoverable from the main display locking repo)

@plinss plinss added the Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review label Jun 7, 2020
@plinss plinss removed this from the 2020-05-21-f2f milestone Jun 7, 2020
@josepharhar
Copy link
Author

Thanks for the feedback! Sorry for the slow reply, I took some time to compare with alternatives more closely and refine the explainer.

This seems intrinsically linked to the content-visibility: hidden-matchable state described in the Display Locking explainer, so it would be helpful to have a bit more discussion on how that state came to be - specifically, why content-visibility: auto isn't sufficient for the "Deep links and searchability into pages with hidden content" case, which this proposal seems primarily concerned with.

beforematch is not intrinsically tied to hidden-matchable. For example, beforematch could be used to expand text hidden inside of <details> elements by firing the event on text hidden inside them without the use of hidden-matchable. On the other hand, hidden-matchable does indeed depend on beforematch.

The case that is handled by content-visibility: hidden-matchable is different from content-visibility: auto. Specifically, the auto case automatically makes the content visible to the user when it is inside the visual viewport (or nearby), without any help from the developer. This would be akin to regular content that appears when scrolled into view. The hidden-matchable case is more similar to the <details> element or mobile Wikipedia sections, where the content is not visible to the user even in the viewport (other than the <summary> content), and requires a different type of user intent to see the content (i.e. click to expand a hidden section).

I struggled to fully understand the use case, even having read through the earlier discussion on #306, this explainer, and the Display Locking explainer mentioned above. Would it be possible to have a clearer description of exactly what problem this is solving?

This event helps with situations such as “collapsed sections of text” or other forms of content that is hidden unless shown by a user interaction (other than scroll). This is similar to the built-in <details> tag, which does not show the contents within it unless the user clicks specifically to open it.

This event allows the developer to provide the ability to search through such content, notifying the page when a match is found, which in turn allows the page to do what is necessary to show the text and reveal the full contents.

One other note: "beforematch is a useful signal to the page which allows custom styling of the matched element, which is now only possible with approximations from scroll positions and intersection observations" - this sounds like a use case that would be better served by a CSS pseudo-selector.

Regarding a pseudo-selector: that’s a good point. Styling a specific element could be achieved with a pseudo-selector. However, we don’t think this approach will work in more complex cases where ancestor or sibling elements need to be re-styled, or interaction with frameworks or virtual scrollers. I added an alternatives section to the explainer to elaborate on this here

Also from the same virtual face-to-face review. My main concern with the proposal is somewhat captured with issue 150. The beforematch event opens the door for re-entrant code introducing easy to achieve circularity. The beforematch event fires >> the event handler removes the target DOM element >> another event handler handles the remove and adds it back to DOM >> etc.

I appreciate that you're upfront listing the issues in your explainer however I didn't see good mitigation besides some ideas around scrolling specific logic. Can you provide more details as to how do you intend to stabilize this?

I do agree that there are some details to work out to mitigate this problem. In our experimentation, the only case we found in a “natural” page is that a section that expands may change layout, so the browser needs to be careful about scroll positioning. In malicious sites (or sites with unfortunate bugs), the cyclic behavior is definitely possible. I would argue that this type of behavior is already possible in most event handlers that observe DOM changes though (examples: MutationObserver, IntersectionObserver, ResizeObserver, scroll event listeners (especially with scroll anchoring)).

One proposal for mitigating this is to allow the user-agent to “give up” after some number of tries. So in the example of find-in-page, this would mean something like

  1. Find a match
  2. Call the beforematch handler
  3. See if the match is where one would expect it to be (e.g. in the same DOM position)
  4. If yes, scroll to it
  5. If no, and there were less than X tries for this particular request, then go back to step 1.

This will be possible because the source of the find-in-page is always a user agent action, not a script-driven action. For that reason also, the spec could suggest the user agent apply such a mitigation but not spell out the mitigation in detail.

@alice
Copy link

alice commented Sep 15, 2020

@josepharhar Sorry for the slow response!

... On the other hand, hidden-matchable does indeed depend on beforematch.

I think some more fleshed out use cases, separated out from the example code, would help guide the review here.

For example, the introduction mentions 'visibility: hidden' subtrees, but that is not covered in a use case. Given 'visibility: hidden' subtrees are usually expected to be hidden from the user in every way, this seems worth expanding on. As an author, when would I use visibility: hidden as opposed to content-visibility: hidden-matchable if I wanted the content to be available to find-in-page in this way? Given visibility: hidden content is hidden from assistive technology, how would this change in behaviour impact assistive technology users?

Similarly, several of the alternatives mention "Doesn't allow "unlocked" hidden-matchable content to become hidden again, which could get confusing." What does this mean? There is no use case which mentions this behaviour, so I'm not clear on how the proposed solution provides this benefit.

Styling a specific element could be achieved with a pseudo-selector. However, we don’t think this approach will work in more complex cases where ancestor or sibling elements need to be re-styled, or interaction with frameworks or virtual scrollers.

Couldn't you use querySelector() to find the match target, then use the same script you would use on the beforeMatch event target? What am I missing here?

@kenchris
Copy link

beforematch is not intrinsically tied to hidden-matchable. For example, beforematch could be used to expand text hidden inside of <details> elements by firing the event on text hidden inside them without the use of hidden-matchable. On the other hand, hidden-matchable does indeed depend on beforematch.

So this is hidden-matchable and <details>, but are there really any other examples? I assume that newer custom elements implementing something like <details> will use content-visibility: hidden-matchable

Regarding a pseudo-selector: that’s a good point. Styling a specific element could be achieved with a pseudo-selector. However, we don’t think this approach will work in more complex cases where ancestor or sibling elements need to be re-styled, or interaction with frameworks or virtual scrollers. I added an alternatives section to the explainer to elaborate on this here

So I see that the cons section lists "There is no way in CSS to say "change my style if a descendant has a pseudo class on it.". I am not a CSS expert but maybe this should be added? @plinss @atanassov

3. See if the match is where one would expect it to be (e.g. in the same DOM position)

How exactly do you do this? If it was collapsed before, how do you know the intended position before laying it out?

@josepharhar
Copy link
Author

Thanks for taking another look at this!

I think some more fleshed out use cases, separated out from the example code, would help guide the review here.

So this is hidden-matchable and <details>, but are there really any other examples? I assume that newer custom elements implementing something like <details> will use content-visibility: hidden-matchable

I have no other examples. The intent of the feature is to allow searching into hidden content, and to allow revealing the content when a match is found. Hidden-matchable and the beforematch event make this possible when used together, and without both of them, we won’t have the desired feature. The details element could also benefit from beforematch since it has revealable hidden content, but it wouldn’t make a ton of sense to have it be the only way to use beforematch.

After thinking about it some more, I agree with these considerations: I think that we should combine the proposals for content-visibility:hidden-matchable and the beforematch event. Should I edit the title and description of this issue? Here is the explainer for hidden-matchable.

As an author, when would I use visibility: hidden as opposed to content-visibility: hidden-matchable if I wanted the content to be available to find-in-page in this way?

The explainer may have been a bit misleading - visibility: hidden text is not and will not be available to find-in-page. I’ll update the explainer to make it less misleading.

Given visibility: hidden content is hidden from assistive technology, how would this change in behaviour impact assistive technology users?

content-visibility: hidden-matchable hides content the same way visibility: hidden does. The native find-in-page feature can fire the beforematch event on hidden-matchable content if a match is found, but we don't think it's possible for other find-in-page mechanisms built on top of the AX tree to fire this event. I don’t have a lot of expertise in assistive technologies, but I think that they don't have full access to the DOM and styles, so they would not be able to fire the beforematch event and scroll like find-in-page does.

Similarly, several of the alternatives mention "Doesn't allow "unlocked" hidden-matchable content to become hidden again, which could get confusing." What does this mean? There is no use case which mentions this behaviour, so I'm not clear on how the proposed solution provides this benefit.

In a previous iteration of this feature, we had content-visibility: hidden-matchable without the beforematch event. When the user searched for text inside of these sections, the section would get revealed without the page being explicitly notified, and the style would remain hidden-matchable. In this case, there is no way for the page to make the selection collapsed again, which is desired in use cases which look similar to a details element where the user clicks on the title or arrow of the collapsible section to toggle the collapsing.
I’ll revise the explainer to make this clearer.

To talk about a specific example, imagine a mobile wikipedia page with collapsed sections. Suppose you find some word, foo, in a collapsed section. This section would have to be revealed in some way. If the user then dismisses the find-in-page dialog, or continues onto the next match, then this section should remain open. However, either the browser makes this state of being expanded sticky (meaning that the script can’t collapse the section anymore), or the section collapses automatically when there is no longer a match (which is not desired behavior). This case can be addressed by firing a beforematch event and letting script design on the correct course of action.

Couldn't you use querySelector() to find the match target, then use the same script you would use on the beforeMatch event target? What am I missing here?

It seems like there isn’t a way to listen for changes in pseudoclasses. This means that it’s unclear when to run this querySelector / script. Even if there was a way you could with a MutationObserver, I think that a DOM event like beforematch is more ergonomic. It would also be challenging to make sure the timing works out well - right now, we make sure that the beforematch event is fired before find-in-page tries to scroll so that script can reveal the content before it gets scrolled to. However, if the page were to try to do this with a MutationObserver, trying to time the find-in-page scroll would be more challenging.

How exactly do you do this? If it was collapsed before, how do you know the intended position before laying it out?

We are only looking at the DOM position, which is independent of the layout/position on screen. If the matching text is removed from the DOM, then we will continue the search and fire the beforematch event again.

@alice
Copy link

alice commented Sep 23, 2020

After thinking about it some more, I agree with these considerations: I think that we should combine the proposals for content-visibility:hidden-matchable and the beforematch event. Should I edit the title and description of this issue? Here is the explainer for hidden-matchable.

Yeah, it seems like these two APIs really form one conceptual API together.

It would make sense to combine the explainers as well, since one doesn't really work without the other. That combined explainer could use the Wikipedia mobile page example as an end to end example illustrating how to use the two APIs in conjunction to create a good user experience.

To talk about a specific example, imagine a mobile wikipedia page with collapsed sections. Suppose you find some word, foo, in a collapsed section. This section would have to be revealed in some way. If the user then dismisses the find-in-page dialog, or continues onto the next match, then this section should remain open. However, either the browser makes this state of being expanded sticky (meaning that the script can’t collapse the section anymore), or the section collapses automatically when there is no longer a match (which is not desired behavior). This case can be addressed by firing a beforematch event and letting script design on the correct course of action.

This is a great example. Are there any other kinds of interactions you had in mind while designing these APIs?

To flesh this out a little more, you might add some example code like:

<h2>Cultural references</h2>
<section class="locked">
Octopus ...
</section>

So what I'm interested in here is the full flow, from loading the page, to matching "Octopus", to the section being closed again. What is the goal state here?

I'm guessing it's something like:

  1. User loads the page. All subsections including "Cultural references" are hidden, with only the headings visible (similar to a <details> element).
  2. User searches for "Octopus"
  3. The <section> contents is made visible to the user
  4. ???
  5. The <section> contents is hidden from the user again.

Is that the flow you're designing for?

If so: what happens at step 4? How does the beforematch event enable it?

Also: at step 3, is it implied that the author must write code to remove the hidden-matchable style, by handling the beforematch event?

@josepharhar
Copy link
Author

It would make sense to combine the explainers as well, since one doesn't really work without the other. That combined explainer could use the Wikipedia mobile page example as an end to end example illustrating how to use the two APIs in conjunction to create a good user experience.

Done: WICG/display-locking#184

This is a great example. Are there any other kinds of interactions you had in mind while designing these APIs?

No, we don't have any other interactions in mind. Collapsible sections is our use case of interest.

To flesh this out a little more, you might add some example code like:

This seems pretty similar to the example code I already have in the explainer, right...?
I am planning on updating the sample code in the explainer to have clickable titles to toggle their corresponding sections, will that help?

Is that the flow you're designing for?

Yes

If so: what happens at step 4? How does the beforematch event enable it?

At least for the wikipedia example and other things that look like details elements, the user would click the heading to collapse the section again. The beforematch event does not specifically enable the behavior of clicking the title to collapse the section.

Also: at step 3, is it implied that the author must write code to remove the hidden-matchable style, by handling the beforematch event?

Yes

@josepharhar
Copy link
Author

After an internal privacy review, we have decided to add some mitigations which restrict the situations where beforematch can be fired. I am updating the explainer here: WICG/display-locking#185
Look for the "Privacy Concerns" section after the PR gets merged.

@josepharhar
Copy link
Author

I just posted about this feature in whatwg and csswg:
whatwg/html#6040
w3c/csswg-drafts#5595

@plinss plinss removed this from the 2020-09-14-week milestone Oct 12, 2020
@plinss plinss added this to the 2020-10-12-week milestone Oct 12, 2020
@kenchris
Copy link

As far as I understand, the user has to reveal the content immediately when a match is found, which might result in content shifting around. Especially I might be typing "icecream" really fast, making a section with the text "ice" show and then unmatch because it didn't contain "icecream". Should there be a bit if grace period, like find-in-page text only triggering an event when the user has made a small pause, like say 50-100ms?

@plinss plinss removed this from the 2020-11-09-week milestone Dec 7, 2020
@atanassov atanassov added this to the 2021-01-25-F2F-Q'onoS milestone Jan 13, 2021
@josepharhar josepharhar changed the title Beforematch event Beforematch event and content-visibility: hidden-matchable Jan 25, 2021
@josepharhar
Copy link
Author

After several discussions at the CSSWG, they are OK with the new content-visibility:hidden-matchable property if the TAG approves it as well.
I updated the explainer to include answers to the issues they brought up:

  • Reader mode
    Should hidden-matchable content be included in reader mode?
    We should recommend it be included because it’s semantically part of the document.
  • Accessibility
    Should hidden-matchable subtrees be included in the accessibility tree?
    No, as Alice said here, because content shouldn’t be included in the accessibility tree until it is actually visible on the page.
  • Which user-agent algorithms should be able to fire the beforematch event on hidden-matchable content?
    Find-in-page, ScrollToTextFragment, and ElementFragments should both fire the beforematch event, and we can add it to their steps in the spec.
  • hidden-matchable content is “part of the current view” but not currently visible on-screen. Is this confusing to developers? (raised by @smfr).
  • Should hidden-matchable be a CSS property or an HTML attribute?
    Many accessibility features are in CSS. For example, the Chromium accessibility tree uses CSS to make decisions about what is visible or not to this tree.
    It's much more ergonomic for developers to use CSS. It's also highly related to the semantics of the existing content-visibility CSS property, so adding an attribute would end up with a matrix of corner cases and a more confusing API.
    It’s also easier to upgrade an existing site with CSS, since they will likely be replacing display:none with content-visibility:hidden-matchable.

As far as I understand, the user has to reveal the content immediately when a match is found, which might result in content shifting around. Especially I might be typing "icecream" really fast, making a section with the text "ice" show and then unmatch because it didn't contain "icecream". Should there be a bit if grace period, like find-in-page text only triggering an event when the user has made a small pause, like say 50-100ms?

I think that the grace period could be implemented later as a separate feature for find-in-page because the same problem already exists without this feature: when you start typing a word, Chrome will try to find everything starting with the very first letter you type, which can slow down the page and/or jump around unexpectedly. Safari, however, has a delay already implemented, and it would exist as part of the browser’s find-in-page UI instead of being something implemented in the rendering engine like this feature is.

@atanassov
Copy link

@kenchris, @alice and myself took another pass reviewing the final proposal of the feature during our "Kronos" VF2F.
Overall we are happy with the current design being a CSS property instead of HTML attribute. The functionality is similar to display:none if we were to add an optional matchable value and make it apply to the contents of the element and not the element itself.

One important question that came up during the review is whether we have verified that this new way of making content hidden is handled correctly per the Name Computation algorithm. Since this feature makes content hidden our expectation is that the behavior will be similar to that of diaplay:none and we ask that you consider adding a WPT test to cover that scenario.

Specifically, that an aria-labelledby or aria-describedby attribute which refers to an element within a content-visibility: hidden, content-visibility: auto or content-visibility: hidden-matchable block will correctly expose the text of the element as contributing to the name/description of the referring element:

 <input aria-labelledby="a-label">
 <div style="content-visibility: hidden-matchable">
   <span id="a-label">This should be the label of the above input</span>
 </div>

Thank you for working with us.

@atanassov atanassov added Resolution: satisfied The TAG is satisfied with this design Venue: CSS WG and removed Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review labels Jan 27, 2021
@josepharhar
Copy link
Author

Thanks for the review!

One important question that came up during the review is whether we have verified that this new way of making content hidden is handled correctly per the Name Computation algorithm. Since this feature makes content hidden our expectation is that the behavior will be similar to that of diaplay:none and we ask that you consider adding a WPT test to cover that scenario.

Specifically, that an aria-labelledby or aria-describedby attribute which refers to an element within a content-visibility: hidden, content-visibility: auto or content-visibility: hidden-matchable block will correctly expose the text of the element as contributing to the name/description of the referring element:

<input aria-labelledby="a-label">
<div style="content-visibility: hidden-matchable">
  <span id="a-label">This should be the label of the above input</span>
</div>

Thanks for the suggestion! I tested out the current implementation in chromium and it works with Mac VoiceOver. I am writing a test here, but unfortunately I don't think there is currently a way to get the appropriate accessibility information in WPT - the test I made uses chromium-specific testing features. I filed a bug on WPT about it here.

@josepharhar
Copy link
Author

FYI this is now an HTML attribute (hidden=until-found) instead of a CSS property (content-visibility:hidden-matchable).
It was proposed by others in the CSSWG here: w3c/csswg-drafts#5595 (comment)
The feature will otherwise be the same, except that the browser will automatically remove the hidden=until-found attribute instead of requiring script to do so.

@josepharhar josepharhar changed the title Beforematch event and content-visibility: hidden-matchable Beforematch event and hidden=until-found Dec 15, 2021
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This was asked for here [1]. Hopefully we can actually upstream it to
WPT once a replacement for content_shell's accessibilityController is
exposed in WPT [2].

[1] w3ctag/design-reviews#511 (comment)
[2] web-platform-tests/wpt#27374

Change-Id: Idc2b610c3b7a52042c3bbecc4689124f78a2be91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2657063
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#848261}
GitOrigin-RevId: 59f6ff500d33ce6f0df48df19348b1ce2445e63e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: satisfied The TAG is satisfied with this design Review type: CG early review An early review of general direction from a Community Group Venue: CSS WG Venue: WHATWG Venue: WICG
Projects
None yet
Development

No branches or pull requests

9 participants