LGTM3

I agree that introspection can be additive on top of what we want to ship
here.

On Wed, Oct 18, 2023 at 5:48 PM Philip Jägenstedt <foo...@chromium.org>
wrote:

> The spec change has now landed, LGTM2.
>
> More introspection could possibly be useful
> <https://github.com/whatwg/html/pull/9462#discussion_r1361110313>, but
> without a concrete use case, example code, or developer feedback, I think
> it's hard to do a good job. Having reviewed the spec change, I'm confident
> that exposing more information is technically straightforward if the need
> arises.
>
> On Thu, Oct 12, 2023 at 1:59 AM Domenic Denicola <dome...@chromium.org>
> wrote:
>
>>
>>
>> On Thu, Oct 12, 2023 at 12:51 AM Yoav Weiss <yoavwe...@chromium.org>
>> wrote:
>>
>>>
>>>
>>> On Friday, October 6, 2023 at 12:34:21 AM UTC+2 Chris Harrelson wrote:
>>>
>>> Hi,
>>>
>>> While Alex's concerns are totally valid to consider from a feature
>>> design perspective, I think they are better to be discussed on the WHATWG
>>> issues for this feature. I chatted offline with Alex and he agreed about
>>> that point, and agreed to post comments and questions there.
>>>
>>> So from an API owners perspective LGTM1 modulo considering and taking
>>> into account all comments and feedback from Alex on the spec (as we should
>>> for all such feedback from anyone, of course!).
>>>
>>>
>>> On Wed, Oct 4, 2023 at 8:28 AM Domenic Denicola <dome...@chromium.org>
>>> wrote:
>>>
>>>
>>>
>>> 2023年10月4日(水) 8:16 Alex Russell <slightly...@chromium.org>:
>>>
>>>
>>>
>>> On Monday, October 2, 2023 at 10:16:53 AM UTC-7 Domenic Denicola wrote:
>>>
>>>
>>>
>>> 2023年10月2日(月) 10:11 Alex Russell <slightly...@chromium.org>:
>>>
>>>
>>>
>>> On Sunday, October 1, 2023 at 9:08:57 PM UTC-7 Domenic Denicola wrote:
>>>
>>> On Sat, Sep 30, 2023 at 5:01 AM Alex Russell <slightly...@chromium.org>
>>> wrote:
>>>
>>> The implicit behaviours based on construction order in this API are very
>>> strange and seem like footguns.
>>>
>>>
>>> I don't understand why you find this strange, or a footgun. It's
>>> intended to be the opposite: it guides developers toward creating the
>>> experience the user expects, where when the user requests to close
>>> something, the last thing that was opened, is what closes.
>>>
>>>
>>> Chris Palmer covered this pretty well recently, so I'll defer to his
>>> more eloquent writeup:
>>>
>>> https://noncombatant.org/2023/05/29/complexities-of-allocation/
>>>
>>> Basically, this is spooky action at a distance and without _at least_
>>> some reflection and manipulation surface (via DOM, probably), it's hard to
>>> understand how this won't turn into a footgun.
>>>
>>> As a separate note, I'm disappointed in the proliferation of APIs that
>>> affect DOM but have no API and reflection. Import Maps spring to mind, but
>>> there are other recent examples too. If manual disposal is going to be
>>> required for this, we should at least make it possible to introspect
>>> outside the scope in which an object that defines this behaviour is
>>> allocated.
>>>
>>>
>>> In what way does this API affect the DOM? No parts of the DOM tree are
>>> modified by CloseWatcher. The same is true for import maps...
>>>
>>>
>>> This is view state, which is frequently reflected via DOM. The primary
>>> concern here is that there's no way to inspect and/or modify the stack
>>> (attached to Node instances or not) independently of closure-scoped object
>>> lifetimes.
>>>
>>>
>>> It's not clear to me what definition of "view state" you are using, such
>>> that it encompasses things like the module specifier resolution algorithm
>>> or the routing of Android back gestures.
>>>
>>> Maybe, if this is a principle you believe in, you could file it as a
>>> suggestion on the w3ctag/design-principles repository, ideally with a clear
>>> explanation of what the boundaries of this "view state" concept are.
>>> (Including what, in your view, would *not* quality as view state.)
>>>
>>>
>>>
>>> The TAG feedback didn't touch on this very much, AFAICT, but it's
>>> somewhat surprising that the stack of close actions isn't inspectable.
>>>
>>>
>>> I can't speak for the TAG, but here are the reasons why the stack of
>>> close watchers isn't inspectable:
>>>
>>>    - We received no developer or partner feedback requesting this
>>>    capability
>>>    - This could cause potential forward-compat problems without careful
>>>    design. E.g., it could make it possible for developers to write code that
>>>    assumes that only CloseWatchers, dialogs, and popover="" elements are 
>>> close
>>>    watchers, and thus make it hard for the web platform to introduce a 
>>> fourth
>>>    close watcher (e.g., <selectlist>) in the future.
>>>    - This would be somewhat of an encapsulation leak between different
>>>    parts of the application, making it harder to write resilient components.
>>>    (This is not a strong argument, but rather a bias toward waiting for a 
>>> use
>>>    case instead of just exposing the information automatically.)
>>>
>>> Thanks, I appreciate the context, and I am impressed by the thoroughness
>>> of the design artifacts.
>>>
>>>
>>> What's the behaviour of non-`destroy()`'d watchers; e.g. if a developer
>>> forgets to dispose of one correctly? Can users get stuck?
>>>
>>>
>>> Non-destroy()ed is the default state of a CloseWatcher, so such
>>> CloseWatchers will respond to the next close request if they are on the top
>>> of the stack. The user cannot really get stuck, as every close request will
>>> either destroy the topmost close watcher on the stack, or possibly trigger
>>> (at most once) a preventDefault()ed cancel event. See
>>> https://github.com/WICG/close-watcher/blob/main/README.md#abuse-analysis
>>> for more details.
>>>
>>>
>>> Also helpful; thank you!
>>>
>>>
>>> Note that the API generally guides you away from this possibility by
>>> making the simpler code be the one that automatically calls destroy() for
>>> you: https://github.com/WICG/close-watcher/blob/main/README.
>>> md#requesting-close-yourself .
>>>
>>>
>>>
>>> On Wednesday, September 27, 2023 at 7:43:49 PM UTC-7 Domenic Denicola
>>> wrote:
>>>
>>> Contact emailsjap...@chromium.org, dome...@chromium.org, jarhar@chro
>>> mium.org
>>>
>>> Explainerhttps://github.com/WICG/close-watcher/blob/main/README.md
>>>
>>> Specificationhttps://github.com/whatwg/html/pull/9462
>>>
>>>
>>> What's preventing the PR from landing?
>>>
>>
>> It needs review, and none of the other editors have made the time yet.
>> (Maybe +Philip Jägenstedt <foo...@chromium.org> could help?)
>>
>>
>>>
>>>
>>>
>>>
>>> Summary
>>>
>>> "Close requests" are a new concept that encompasses user requests to
>>> close something currently open, using the Esc key on desktop or the back
>>> gesture/button on Android. Integrating them into Chromium comes with two
>>> changes: * CloseWatcher, a new API for directly listening and responding to
>>> close requests. * Upgrades to <dialog> and popover="" to use the new close
>>> request framework, so that they respond to the Android back button.
>>>
>>>
>>> Blink componentBlink
>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink>
>>>
>>> TAG reviewhttps://github.com/w3ctag/design-reviews/issues/594
>>>
>>> TAG review statusIssues addressed
>>>
>>> Risks
>>>
>>>
>>> Interoperability and Compatibility
>>>
>>> This API is designed to have an interoperable surface for web
>>> developers, to help them avoid platform-specific code. So, if it were
>>> implemented across browsers, it would be a positive for interoperability.
>>> Otherwise, it has the usual risks of not getting adopted by other vendors.
>>> Compatibility: To avoid allowing CloseWatchers, dialogs, and popovers
>>> ("close watchers") to prevent the Android back gesture/button from
>>> navigating through history, how close watchers respond to close requests
>>> depends on user activation. If no user activation occurs between opening,
>>> and the user issuing a close request, this can cause a
>>> CloseWatcher/dialog's cancel event to be skipped, or cause multiple close
>>> watchers to be closed at once. Although this behavior is meant to prevent
>>> back-trapping on Android specifically, it applies to desktop as well, for
>>> interoperability reasons. This change is a compatibility risk. However, use
>>> counters show it to be an acceptable one: - 0.000015% of pages impacted by
>>> skipped cancel events - 0.000007% of pages impacted by skipped cancel
>>> events that would otherwise call preventDefault() - between 0.000000% and
>>> 0.000001% of pages impacted by multiple dialogs closed
>>>
>>>
>>> *Gecko*: Positive (https://github.com/mozilla/st
>>> andards-positions/issues/604)
>>>
>>> *WebKit*: No signal (https://github.com/WebKit/sta
>>> ndards-positions/issues/215)
>>>
>>> *Web developers*: Positive (https://github.com/w3ctag/des
>>> ign-reviews/issues/594#issuecomment-890257686) See also
>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1319915
>>>
>>> *Other signals*:
>>>
>>> Activation
>>>
>>> The CloseWatcher API is meant to be usable as a progressive enhancement;
>>> if developers use it with feature detection, then their app will be able to
>>> watch for unusual close watchers in supporting browsers, while falling back
>>> to listening for the Esc key in browsers that haven't implemented the API.
>>> It would benefit from a conditional polyfill that translates the Esc key
>>> into a close signal, so that then developers don't even have to have
>>> feature detection and fallback logic, but can just use the CloseWatcher API
>>> surface. One such polyfill is available in the demo:
>>> https://close-watcher-demo.glitch.me/
>>>
>>>
>>> Security
>>>
>>> The main security-related concern in this API is preventing it from
>>> being usable for back-trapping, i.e. disabling the Android back
>>> gesture/button. Although this is already possible in Chromium and other
>>> browsers due to bugs, we worked to ensure CloseWatcher and close request
>>> integration to dialogs/popups does not increase the size of the problem, by
>>> gating repeated use of these behind transient user activation checks: see
>>> https://github.com/WICG/close-watcher#abuse-analysis
>>>
>>>
>>> WebView application risks
>>>
>>> Does this intent deprecate or change behavior of existing APIs, such
>>> that it has potentially high risk for Android WebView-based applications?
>>>
>>> Beyond the low risks already listed in the Compat section, we do not
>>> anticipate any WebView-specific risks. A base::Feature killswitch is
>>> available just in case.
>>>
>>>
>>> Debuggability
>>>
>>> No special DevTools support is required.
>>>
>>>
>>> Will this feature be supported on all six Blink platforms (Windows, Mac,
>>> Linux, Chrome OS, Android, and Android WebView)?Yes
>>>
>>> Is this feature fully tested by web-platform-tests
>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md>
>>> ?Yes
>>>
>>> Flag name on chrome://flagsCloseWatcher
>>>
>>> Finch feature nameCloseWatcher
>>>
>>> Requires code in //chrome?False
>>>
>>> Tracking bughttps://bugs.chromium.org/p/chromium/issues/detail?id=117
>>> 1318
>>>
>>> Non-OSS dependencies
>>>
>>> Does the feature depend on any code or APIs outside the Chromium open
>>> source repository and its open-source dependencies to function?
>>> No.
>>>
>>> Sample links
>>> https://close-watcher-demo.glitch.me
>>>
>>> Estimated milestonesShipping on desktop119DevTrial on desktop97Shipping
>>> on Android119DevTrial on Android97Shipping on WebView119
>>>
>>> Anticipated spec changes
>>>
>>> Open questions about a feature may be a source of future web compat or
>>> interop issues. Please list open issues (e.g. links to known github issues
>>> in the project for the feature specification) whose resolution may
>>> introduce web compat/interop risk (e.g., changing to naming or structure of
>>> the API in a non-backward-compatible way).
>>> None.
>>>
>>> Link to entry on the Chrome Platform Statushttps://chromestatus.com/
>>> feature/4722261258928128
>>>
>>> Links to previous Intent discussionsIntent to prototype: https://groups.
>>> google.com/a/chromium.org/g/blink-dev/c/NA5NC16OmsU
>>>
>>> This intent message was generated by Chrome Platform Status
>>> <https://chromestatus.com/>.
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "blink-dev" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to blink-dev+unsubscr...@chromium.org.
>>> To view this discussion on the web visit https://groups.google.com/a/
>>> chromium.org/d/msgid/blink-dev/CAM0wra-SULEU6D-HmDDuf%
>>> 3D5T9faNVk_LcqjKxY%3Do%3Du-vqTzaag%40mail.gmail.com
>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM0wra-SULEU6D-HmDDuf%3D5T9faNVk_LcqjKxY%3Do%3Du-vqTzaag%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>>

-- 
You received this message because you are subscribed to the Google Groups 
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfUNr-P-OMJD4ZFogg%3DZYT%2B9R-AtnuRTJiM7RavJRy8E3A%40mail.gmail.com.

Reply via email to