Contact emails
lan...@chromium.org, bokan@chromium.org, nzolghadr@chromium.org
Summary
Right now in Blink, in order to update the hover state when the page scrolls or the page layout is changed, we have to dispatch fake mouse moves based on a timer, because the real mouse does not move. However, every mouse move does a hit test, which is an issue for performance as well as testing. The proposal is to apply the hover effect when the mouse physically moves.
Motivation
The primary motivation is to improve the performance and dispatching the fake mouse events makes testing difficult and increases complexity for our developers. The hit tests of the fake mouse events really slow down the performance. In addition, whether updating the hover state when the mouse does not move is desirable from a UX standpoint is debatable, and the fake mouse events cause many issues to web developers and testing.
Interoperability and Compatibility risk
Yes. Firefox and Safari update the hover state when the page layout is changed and after scroll. Edge does not update the hover state when the page layout is changed, but it updates hover after scroll.
Will this feature be supported on all six Blink platforms (Windows, Mac, Linux,
Chrome OS, Android, and Android WebView)?
Yes
Demo link
https://output.jsbin.com/cekolep/quiet/
https://www.bitdegree.org/learn/best-code-editor/?example=26436
OWP launch tracking bug
Link to entry on the Chrome Platform Status
https://www.chromestatus.com/feature/6058736774807552
Requesting approval to ship
no
--
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+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2BEkLDjvH4ZNgsjSXucQq4y_R0M9BNwXRbyTj3rzrDJewaTU3A%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw98ob0W_vPTgm%2BU3VqmhEJdMMVtSpYVWEYhoYsw37wDfA%40mail.gmail.com.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/414aca9a-5aa1-4c3c-9532-17af868ff1fe%40chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABc02_%2Bn9JK1Q9p91mJNKi%2B1Gz74OERBMbEUjsVYsr552tJmBA%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAB8%3DuKamtYMSZUfjiZZ6b6OcH%2Bv-h1czRHHYm%2BjOYv%3DgcOzYbQ%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHgVhZV%2BZR%2BjTzeQDkiD_M5jmvbF%2BBxEeYequLU2ksg5U22G5A%40mail.gmail.com.
It was related to specing this intent. I'm curious what the motivation here is. We specifically decided before that the mouse events sent should always match the hover states. Being that I've been out of the loop on input things I'm curious on the motiviation of this. Was the previous intent flawed? Where there drawbacks? Why are we deviating from Firefox here?
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHgVhZVfOrEq6%2B43fY7PRkw-F10WPTz9JonFTQ9D1rrBoeVQUg%40mail.gmail.com.
On Thu, Sep 6, 2018 at 10:09 AM Dave Tapuska <dtap...@chromium.org> wrote:It was related to specing this intent. I'm curious what the motivation here is. We specifically decided before that the mouse events sent should always match the hover states. Being that I've been out of the loop on input things I'm curious on the motiviation of this. Was the previous intent flawed? Where there drawbacks? Why are we deviating from Firefox here?To me the primary motivation is performance. If the fake mouse event timer happens when the document lifecycle is dirty, Blink has to perform a forced style and layout which otherwise would not have been necessary. Then it performs a hit test to determine the new elements under the mouse. This in turn can cause a full lifecycle when hover state changes (which may actually change hover due to layout moving elements out from the mouse right? - a rendering circularity). The link provided earlier in the thread (here) goes into some more cases of this performance problem. The hacks described there are the only way to avoid this performance problem as far as I can see. In their case, scroll is being optimized, but UIs which want to have hover state but don't use scroll for certain interactions will also suffer performance problems.
On Thursday, September 6, 2018 at 10:54:10 AM UTC-7, Chris Harrelson wrote:On Thu, Sep 6, 2018 at 10:09 AM Dave Tapuska <dtap...@chromium.org> wrote:It was related to specing this intent. I'm curious what the motivation here is. We specifically decided before that the mouse events sent should always match the hover states. Being that I've been out of the loop on input things I'm curious on the motiviation of this. Was the previous intent flawed? Where there drawbacks? Why are we deviating from Firefox here?To me the primary motivation is performance. If the fake mouse event timer happens when the document lifecycle is dirty, Blink has to perform a forced style and layout which otherwise would not have been necessary. Then it performs a hit test to determine the new elements under the mouse. This in turn can cause a full lifecycle when hover state changes (which may actually change hover due to layout moving elements out from the mouse right? - a rendering circularity). The link provided earlier in the thread (here) goes into some more cases of this performance problem. The hacks described there are the only way to avoid this performance problem as far as I can see. In their case, scroll is being optimized, but UIs which want to have hover state but don't use scroll for certain interactions will also suffer performance problems.Removing the timer-based checks and switching to a triggered model - shortly after layout changes, and on scroll quiescence + a low-res timer during scroll - makes more sense if the goal is to improve performance.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/565caff4-8c98-45b1-ba3b-92d1d9dc46a1%40chromium.org.
--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/affb09a2-a8a1-09db-65e5-38fa8233d3c3%40mit.edu.
Motivation
In addition, whether updating the hover state when the mouse does not move is desirable from a UX standpoint is debatable,
We have submitted the changes that we will not dispatch the fake mouse move based on a timer, but we will update the hover state once the scroll finishes.
We have submitted the changes that we will not dispatch the fake mouse move based on a timer, but we will update the hover state once the scroll finishes.
The intent and the discussion in the bug makes it sound like you want to remove the timer entirely for all style and layout changes. The RuntimeEnabledFeatures::NoHoverAfterLayoutChangeEnabled() flag currently also disables the update and the timer entirely not just for scroll.
The timer is needed because it's not safe to run script inside style or layout, so when the element under the mouse changes a timer is used to safely dispatch the events later. It also provides natural queuing because thrashy style or layout only triggers one hit test.
I don't think RuntimeEnabledFeatures::NoHoverAfterLayoutChangeEnabled() is a good idea from a UX perspective, and firing the events synchronously isn't safe.
On Wednesday, November 7, 2018 at 10:51:22 PM UTC-5, Elliott Sprehn wrote:The intent and the discussion in the bug makes it sound like you want to remove the timer entirely for all style and layout changes. The RuntimeEnabledFeatures::NoHoverAfterLayoutChangeEnabled() flag currently also disables the update and the timer entirely not just for scroll.There are two flags as part of this change, one is the style+layout hover update, NoHoverAfterLayoutChangeEnabled. The other is scroll hover update behind NoHoverDuringScrollEnabled. The proposed non-intent-to-ship change above is for the scrolling case only, synchronously performing the hover update in a GestureScrollEnd should be ok. One thing I hadn't thought of earlier though is the flag doesn't yet work for programmatic scrolls and non-gesture scrolls like keyboard so we'd want to implement that before shipping it.The timer is needed because it's not safe to run script inside style or layout, so when the element under the mouse changes a timer is used to safely dispatch the events later. It also provides natural queuing because thrashy style or layout only triggers one hit test.The intent is to do it at a safe time inside the lifecycle, not inside style/layout itself. JS execution already happens inside the lifecycle after a layout, for example, see: Document::LayoutUpdated. Perhaps it'd make sense to queue the DOM events until the next raf.
The difficulty with using timers/postTask for this is it makes things like tests less predictable and harder to write (IIRC, pdr@ was bitten by this recently which spurred this project). It also causes scheduling surprises, e.g. 855576. These would just go away if we integrated better with the lifecycle.I don't think RuntimeEnabledFeatures::NoHoverAfterLayoutChangeEnabled() is a good idea from a UX perspective, and firing the events synchronously isn't safe.Agreed, we don't want to change the existing UX.
On Thu, Nov 8, 2018 at 10:09 AM bokan via blink-dev <blin...@chromium.org> wrote:On Wednesday, November 7, 2018 at 10:51:22 PM UTC-5, Elliott Sprehn wrote:The intent and the discussion in the bug makes it sound like you want to remove the timer entirely for all style and layout changes. The RuntimeEnabledFeatures::NoHoverAfterLayoutChangeEnabled() flag currently also disables the update and the timer entirely not just for scroll.There are two flags as part of this change, one is the style+layout hover update, NoHoverAfterLayoutChangeEnabled. The other is scroll hover update behind NoHoverDuringScrollEnabled. The proposed non-intent-to-ship change above is for the scrolling case only, synchronously performing the hover update in a GestureScrollEnd should be ok. One thing I hadn't thought of earlier though is the flag doesn't yet work for programmatic scrolls and non-gesture scrolls like keyboard so we'd want to implement that before shipping it.The timer is needed because it's not safe to run script inside style or layout, so when the element under the mouse changes a timer is used to safely dispatch the events later. It also provides natural queuing because thrashy style or layout only triggers one hit test.The intent is to do it at a safe time inside the lifecycle, not inside style/layout itself. JS execution already happens inside the lifecycle after a layout, for example, see: Document::LayoutUpdated. Perhaps it'd make sense to queue the DOM events until the next raf.Ah yeah I see asserts were disabled here: https://chromium.googlesource.com/chromium/src/+/dbd4eed1920a2aafbc87236db7efb98f108ba063The ScriptForbiddenScope was there to guard against system sanity and spec violations. The timers and async post layout tasks are required to maintain invariants and spec compliance. Previously only plugins could run script inside layout and it was considered a bug. Plugins also don't exist on mobile, and flash is on the way out. I think we also block the script in plugins in a bunch of situations so those comments are probably out of date.That change also breaks some specs. For example that change means that events dispatch inside .offsetTop now which isn't allowed, and is very confusing as an author. It also means bad frames are painted when requestAnimationFrame() is called inside a relevant event handler. For example: onfocus = () => { body.style.color = 'red'; requestAnimationFrame(() => body.style.color = 'blue') } that must paint blue per spec, but it'll paint red for one frame with the change that runs script inside layout. Raf callbacks must always be last.I understand the timers are annoying, but I think you probably want to revert those changes and expand the ScriptForbiddenScope to cover the whole function again. The change discussed in https://bugs.chromium.org/p/chromium/issues/detail?id=796222 is not safe or spec compliant.