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

spark.adobe.com - Page scroll is slow #89116

Closed
shapiro125 opened this issue Oct 5, 2021 · 8 comments
Closed

spark.adobe.com - Page scroll is slow #89116

shapiro125 opened this issue Oct 5, 2021 · 8 comments
Labels
browser-firefox engine-gecko The browser uses the Gecko rendering engine os-mac Issues only happening on macOS. priority-critical priority-normal severity-important A non-core broken piece of functionality, not behaving the way you would expect. type-installTrigger issues related to window.installTrigger
Milestone

Comments

@shapiro125
Copy link

URL: https://spark.adobe.com/page/0OAACYUYlqxNL/

Browser / Version: Firefox 93.0
Operating System: Mac OS X 10.15
Tested Another Browser: Yes Chrome

Problem type: Something else
Description: Scrolling very slow versus Chrome
Steps to Reproduce:
Scrolling down the page in Firefox is very slow on Adobe Spark. It seems to move down the page at less "clicks" (for a better word) and much less smooth than Chrome.

Browser Configuration
  • None

From webcompat.com with ❤️

@webcompat-bot webcompat-bot added this to the needstriage milestone Oct 5, 2021
@webcompat-bot webcompat-bot added browser-firefox engine-gecko The browser uses the Gecko rendering engine priority-critical labels Oct 5, 2021
@softvision-raul-bucata softvision-raul-bucata added the os-mac Issues only happening on macOS. label Oct 5, 2021
@shapiro125
Copy link
Author

Here is a screen recording

Screen.Recording.2021-10-05.at.9.57.54.AM.mp4

.

@softvision-oana-arbuzov
Copy link
Member

Thanks for the report, I was able to reproduce the issue. Scrolling is slower on Firefox compared to Chrome.
image

Note: The issue is not reproducible on Chrome.

I've recorded a performance profile: https://share.firefox.dev/3iB2Qux

Tested with:
Browser / Version: Firefox Nightly 95.0a1 (2021-10-05)
Operating System: Windows 10 Pro

Moving to Needsdiagnosis for further investigation.

@softvision-oana-arbuzov softvision-oana-arbuzov changed the title spark.adobe.com - see bug description spark.adobe.com - Page scroll is slow Oct 6, 2021
@softvision-oana-arbuzov softvision-oana-arbuzov added priority-normal severity-important A non-core broken piece of functionality, not behaving the way you would expect. labels Oct 6, 2021
@denschub
Copy link
Member

denschub commented Oct 6, 2021

So, for some reason, that site decided to completely implement their own scrolling. Even the "scrollbar" visible on the side is just a bunch of HTML with a draggable element on it... Their mousewheel handler has this if condition in it, which is... quite something:

if (b.isSafari ? 'deltaY' in h || (k = Math[k >= 1 ? 'floor' : 'ceil'](k / 3), m = Math[m >= 1 ? 'floor' : 'ceil'](m / 3), n = Math[n >= 1 ? 'floor' : 'ceil'](n / 3)) : b.isFirefox ? 0 === h.deltaMode && (k = Math[k >= 1 ? 'floor' : 'ceil'](k / 2), m = Math[m >= 1 ? 'floor' : 'ceil'](m / 2), n = Math[n >= 1 ? 'floor' : 'ceil'](n / 2)) : b.isChrome || b.isIE || b.isMSEdge || (o = Math.max(Math.abs(n), Math.abs(m)), (!g || o < g) && (g = o, e(h, o) && (g /= 40)), e(h, o) && (k /= 40, m /= 40, n /= 40), k = Math[k >= 1 ? 'floor' : 'ceil'](k / g), m = Math[m >= 1 ? 'floor' : 'ceil'](m / g), n = Math[n >= 1 ? 'floor' : 'ceil'](n / g)), l.settings.normalizeOffset && this.getBoundingClientRect) {
  /* ... */
}

which expands to something like

if (
  b.isSafari ? 'deltaY' in h || (
    k = Math[k >= 1 ? 'floor' : 'ceil'](k / 3);
    m = Math[m >= 1 ? 'floor' : 'ceil'](m / 3);
    n = Math[n >= 1 ? 'floor' : 'ceil'](n / 3);
  )
  : b.isFirefox ? 0 === h.deltaMode && (
    k = Math[k >= 1 ? 'floor' : 'ceil'](k / 2);
    m = Math[m >= 1 ? 'floor' : 'ceil'](m / 2);
    n = Math[n >= 1 ? 'floor' : 'ceil'](n / 2);
  )
  : b.isChrome || b.isIE || b.isMSEdge || (
    o = Math.max(Math.abs(n), Math.abs(m));
    (!g || o < g) && (g = o, e(h, o) && (g /= 40));
    e(h, o) && (k /= 40, m /= 40, n /= 40);
    k = Math[k >= 1 ? 'floor' : 'ceil'](k / g);
    m = Math[m >= 1 ? 'floor' : 'ceil'](m / g);
    n = Math[n >= 1 ? 'floor' : 'ceil'](n / g);
  ), l.settings.normalizeOffset && this.getBoundingClientRect
)

and that's interesting. They correctly expect that deltaMode is DOM_DELTA_PIXEL for Firefox, which is a "recent-ish" change. Interestingly, the way they calculate the distance to scroll differs between Firefox and Chrome/Edge. I don't fully understand why they do what they do, if I'm being honest.

Spoofing as Chrome, and thus running in the same codepath here, makes the scrolling behave the same across Firefox and Chrome. Unfortunately, "spoofing" is not as simple as switching the User Agent here, we'd have to undefine window.InstallTrigger and set window.chrome to something truthy to make their browser detection think we're Chrome. I can't easily test this right now, but it might be still possible to ship an intervention if nothing else breaks. I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1734432 to see what I can do.

A search on LinkedIn told me that @senocular works on the frontend for Adobe Spark. Hi Trevor! Sorry for the ping, but could you maybe take a look at why scrolling on this page behaves differently in Firefox vs. the other browsers? :)

@denschub denschub modified the milestones: needsdiagnosis, sitewait Oct 6, 2021
@denschub denschub added the action-needssitepatch This web bug needs a GoFaster site patch. label Oct 6, 2021
@denschub denschub removed their assignment Oct 6, 2021
@senocular
Copy link

I don't work on Page anymore, but I do know that code is quite old and I believe originated with a jquery plugin. But looking at how both browsers handle the wheel event, it looks like it has more to do (from what I'm seeing) with how acceleration is being treated. Using:

window.addEventListener('wheel', e => console.log(e.deltaY), true)

On Mac Firefox I'm seeing deltas between 16 and 160 whereas on Chrome I'm seeing between 4 and 380. Firefox ends up being faster per "click" but doesn't ramp up as much as Chrome when you're slapping that wheel wheely hard.

@denschub
Copy link
Member

denschub commented Oct 6, 2021

Hm, interesting. I only have access to a laptop touchpad and an Apple Magic Trackpad at the moment as my mouse is waiting on replacement... And for touchpads (and touchscreens), the behvaior ends up feeling the same if we get into the Chrome codepath.

@karlcow didn't you recently look into wheel events and the delta values? Are you aware of any differences in how we transfer acceleration?

@karlcow
Copy link
Member

karlcow commented Oct 7, 2021

acceleration is set here.
https://searchfox.org/mozilla-central/rev/a9ef6ad97d2d5f96d5ed51eda38f1a02700ccff7/modules/libpref/init/StaticPrefList.yaml#8852-8865

And there is code here on computing the acceleration.
https://searchfox.org/mozilla-central/rev/a9ef6ad97d2d5f96d5ed51eda38f1a02700ccff7/dom/events/WheelHandlingHelper.cpp#359-400

This line is quite interesting, specifically because of the recent changes.

  // Don't accelerate the delta values if the event isn't line scrolling.
  if (aEvent->mDeltaMode != dom::WheelEvent_Binding::DOM_DELTA_LINE) {
    return result;
  }

Does that mean Firefox does not accelerate if it's DOM_DELTA_PIXEL
This is code written by Masayuki-san @masayuki-nakano

ah but the logic of the code predates this.
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&ignore=&bug=984271&attachment=8399331

hmm but wait I thought this code was modified by @emilio too.
ah yes in https://bugzilla.mozilla.org/show_bug.cgi?id=1708829

ok @emilio do your modifications have an impact on acceleration?

@masayuki-nakano
Copy link

acceleration is set here. https://searchfox.org/mozilla-central/rev/a9ef6ad97d2d5f96d5ed51eda38f1a02700ccff7/modules/libpref/init/StaticPrefList.yaml#8852-8865

And there is code here on computing the acceleration. https://searchfox.org/mozilla-central/rev/a9ef6ad97d2d5f96d5ed51eda38f1a02700ccff7/dom/events/WheelHandlingHelper.cpp#359-400

This line is quite interesting, specifically because of the recent changes.

This method is for computing scroll amount of default action of wheel events. So it's not related to any information exposed to web apps. It was designed for making faster scroll on Windows, but it's good only for specific environments, so IIRC, it's not used by default.

According to @denschub 's investigation, it looks like that Adobe tried to do something for Firefox for macOS with high-precision wheel pointing device such as Magic Trackpad or expensive mouse. If now Adobe refers deltaY before the if, most environments are run in the path newly.

@karlcow karlcow added the type-installTrigger issues related to window.installTrigger label Mar 11, 2022
@denschub
Copy link
Member

I'm closing this as a WONTFIX. The reasoning for that is explained on Bugzilla, and I'll full-quote here for reference:

Okay, I'm not only going to de-scope this for 103, I'm actually closing this as a wontfix.

For context: the issue we're trying to solve here is not a critical breakage. Instead, it's just about the scrolling being slower than expected. This is ultimately due to a Firefox-only codepath that handles the scroll event differently. It's certainly a bit irritating, but that's about it.

Shimming InstallTrigger and window.chrome works to "fix" the scrolling issue. As in, it's scrolling faster with that, and scrolling is consistent with Chrome - but that scrolling is still a custom scrolling logic, and with the workaround in place, it actually feels too fast as compared to the rest of web content in Firefox.

When spoofing as Chrome, there are a bunch of exceptions and potential code-paths that worry me a bit. And since Adobe Express (the new name for Spark) is a site builder with a large number of sites in it, we can't possibly test all the sites to make sure this doesn't break another site.

Ultimately, this would be a medium-risk patch for a non-breaking "this just feels weird" UX, that would still feel weird - just a different kind of weird - with the intervention.

@denschub denschub closed this as not planned Won't fix, can't repro, duplicate, stale Jun 23, 2022
@denschub denschub modified the milestones: sitewait, wontfix Jun 23, 2022
@denschub denschub removed the action-needssitepatch This web bug needs a GoFaster site patch. label Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-firefox engine-gecko The browser uses the Gecko rendering engine os-mac Issues only happening on macOS. priority-critical priority-normal severity-important A non-core broken piece of functionality, not behaving the way you would expect. type-installTrigger issues related to window.installTrigger
Projects
None yet
Development

No branches or pull requests

8 participants