WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
247445
Please remove a desmos.com quirks entry
https://bugs.webkit.org/show_bug.cgi?id=247445
Summary
Please remove a desmos.com quirks entry
Jason Merrill
Reported
2022-11-03 10:41:26 PDT
desmos.com is currently targeted in Quirks.cpp:
https://github.com/WebKit/WebKit/blob/a6c922ea6f0526752da82adfad89afd1ef26268a/Source/WebCore/page/Quirks.cpp#L442-L443
The quirk test requires the url to include `/calculator/`, but the graphing calculator start page does not include a trailing slash (it is
https://www.desmos.com/calculator
). URLs for saved graphs do include `/calculator/` in the path (example:
https://www.desmos.com/calculator/2rnqgoa6a4
). Desmos recently ran into a bug that was only showing up in saved graphs, and not for the blank calculator. We eventually traced the difference back to this quirk. I don't believe this quirk is required anywhere on Desmos anymore, and in any case, it was not being applied to the most commonly used calculator page. Please remove it.
Attachments
Add attachment
proposed patch, testcase, etc.
Jason Merrill
Comment 1
2022-11-03 10:43:20 PDT
As an aside, is there anything sites can do to opt out of certain quirks? Even if this quirk is removed, I'm wondering if there's anything Desmos can do to make sure it does not affect us on previous releases of the browser.
Karl Dubost
Comment 2
2022-11-06 16:00:20 PST
Hi Jason, First of all, thanks a lot for reporting the issue. Could you give a bit more information on 1. the steps to reproduce the issue 2. which device this is happening. 3. which version of the OS and Safari. :) (I suspect iOS or iPadOS) Digging through the code There are two quirks currently
https://github.com/WebKit/WebKit/blob/4718faa509f327097a30847ee21ede844aff6670/Source/WebCore/page/Quirks.cpp#L541-L543
```cpp std::optional<Event::IsCancelable> Quirks::simulatedMouseEventTypeForTarget(EventTarget* target) const { if (!shouldDispatchSimulatedMouseEvents(target)) return { }; // cut for brevity auto host = m_document->topDocument().url().host(); if (equalLettersIgnoringASCIICase(host, "desmos.com"_s) || host.endsWithIgnoringASCIICase(".desmos.com"_s)) return Event::IsCancelable::No; // cut for brevity return Event::IsCancelable::Yes; } ``` and
https://github.com/WebKit/WebKit/blob/4718faa509f327097a30847ee21ede844aff6670/Source/WebCore/page/Quirks.cpp#L442-L443
```cpp bool Quirks::shouldDispatchSimulatedMouseEvents(const EventTarget* target) const { if (DeprecatedGlobalSettings::mouseEventsSimulationEnabled()) return true; if (!needsQuirks()) return false; auto doShouldDispatchChecks = [this] () -> ShouldDispatchSimulatedMouseEvents { auto* loader = m_document->loader(); if (!loader || loader->simulatedMouseEventsDispatchPolicy() != SimulatedMouseEventsDispatchPolicy::Allow) return ShouldDispatchSimulatedMouseEvents::No; // cut for brevity auto& url = m_document->topDocument().url(); auto host = url.host().convertToASCIILowercase(); // cut for brevity if ((host == "desmos.com"_s || host.endsWith(".desmos.com"_s)) && startsWithLettersIgnoringASCIICase(url.path(), "/calculator/"_s)) return ShouldDispatchSimulatedMouseEvents::Yes; // cut for brevity return ShouldDispatchSimulatedMouseEvents::No; }; // cut for brevity ASSERT_NOT_REACHED(); return false; } ``` ## First Quirk The first one was started on 2019-04-11
Bug 196830
<
rdar://problem/49124313
>
https://github.com/WebKit/WebKit/commit/a2839dea43fe5b86b6d5ec6132137dd462ed7094
and then adjusted on 2019-05-07
Bug 197652
<
rdar://problem/47068176
>
https://github.com/WebKit/WebKit/commit/721679dc53a16b6ba9a05b5b4f5b993f9f4cf991
This is enabled only if Touch events are active.
https://github.com/WebKit/WebKit/blob/4718faa509f327097a30847ee21ede844aff6670/Source/WebCore/page/Quirks.h#L68-L69
The initial steps to reproduce were: Steps: 1. Visit
https://www.desmos.com/calculator
2. Touch the graph screen to move the graph around 3. Touch the formula field 4. Type keys on soft or HW keyboard Expected Results: Characters are entered into formula field Actual Results: No characters are entered into formula field ## Second Quirk The second one was started on 2019-07-04
Bug 199508
<
rdar://problem/50925173
>
https://github.com/WebKit/WebKit/commit/9d226cb633d0dd7379f5de1bc6c245e8f583cc5c
Steps 1. Load
https://www.desmos.com/calculator/vymzvpqp3k
or any sheet with more formulas than will fit on screen. 2. Swipe to scroll formula Results: Formula region does not scroll.
Karl Dubost
Comment 3
2022-11-06 16:12:08 PST
Jason, ## Disabling Site Quirks Safari Desktop has a "Disable Site Specific Hacks" in the Develop menu, but not Safari on iPad it seems. So indeed that makes it a bit harder to test. I will test in a build without the Quirks lines to double check. Maybe I missed it, but just in case to not forget.
Bug 247555
## Opting Out of Quirks for a Website
> Is there anything sites can do to opt out of certain quirks? Even if this quirk is removed, I'm wondering if there's anything Desmos can do to make sure it does not affect us on previous releases of the browser.
This is a very interesting question. The current answer is no. But the idea of a kill switch for a site interventions for older versions of the browsers is interesting. If such feature existed, what would be the way to trigger it on your side?
Karl Dubost
Comment 4
2022-11-07 15:12:16 PST
Jason, to deactivate the Quirks, you could connect the external device to a Mac and with the Web Inspector deactivate it. See
https://webkit.org/web-inspector/device-settings/
Jason Merrill
Comment 5
2022-11-09 14:05:37 PST
(In reply to Karl Dubost from
comment #2
) My eventual goal is to have all site specific quirks for desmos.com removed. Desmos licenses our calculator products to partners to embed in other domains, and this is a very important part of our business. Any place that the calculator does not function correctly outside of desmos.com is a problem for us and our partners. But I would like to focus this issue on what you describe as "Quirk 2" because I think that one may be easiest to deal with.
> Could you give a bit more information on > > 1. the steps to reproduce the issue
This is tricky because I think Desmos is currently set up to function correctly either with or without Quirk 1 in effect. We have observed that it changes which mouse and touch events are fired, but there's no user facing way to observe this that we know of. You'd have to add extra event listeners with logging to easily see this.
> 2. which device this is happening. > 3. which version of the OS and Safari. :) (I suspect iOS or iPadOS)
We have observed the effects of this quirk on two different iPads, one running iPadOS 15.6.1, and another running iPadOS 16.
> > Digging through the code > > There are two quirks currently >
https://github.com/WebKit/WebKit/blob/
> 4718faa509f327097a30847ee21ede844aff6670/Source/WebCore/page/Quirks.cpp#L541- > L543 >
[snip]
> > and >
https://github.com/WebKit/WebKit/blob/
> 4718faa509f327097a30847ee21ede844aff6670/Source/WebCore/page/Quirks.cpp#L442- > L443 > > ```cpp > bool Quirks::shouldDispatchSimulatedMouseEvents(const EventTarget* target) > const > { > if (DeprecatedGlobalSettings::mouseEventsSimulationEnabled()) > return true; > > if (!needsQuirks()) > return false; > > auto doShouldDispatchChecks = [this] () -> > ShouldDispatchSimulatedMouseEvents { > auto* loader = m_document->loader(); > if (!loader || loader->simulatedMouseEventsDispatchPolicy() != > SimulatedMouseEventsDispatchPolicy::Allow) > return ShouldDispatchSimulatedMouseEvents::No; > > // cut for brevity > > auto& url = m_document->topDocument().url(); > auto host = url.host().convertToASCIILowercase(); > > // cut for brevity > > if ((host == "desmos.com"_s || host.endsWith(".desmos.com"_s)) && > startsWithLettersIgnoringASCIICase(url.path(), "/calculator/"_s)) > return ShouldDispatchSimulatedMouseEvents::Yes; > > // cut for brevity > > return ShouldDispatchSimulatedMouseEvents::No; > }; > > // cut for brevity > > ASSERT_NOT_REACHED(); > return false; > } > ```
I think there's an easy case to make that this quirk should be removed: it only targets paths starting with "/calculator/" (note the trailing slash), and so it is not targeting the most common URL for using the calculator (
https://www.desmos.com/calculator
) (note, no trailing slash).
> > ## First Quirk >
[snip to instead focus on the second quirk]
> > ## Second Quirk > > The second one was started on 2019-07-04 >
Bug 199508
> <
rdar://problem/50925173
> >
https://github.com/WebKit/WebKit/commit/
> 9d226cb633d0dd7379f5de1bc6c245e8f583cc5c > > Steps > 1. Load
https://www.desmos.com/calculator/vymzvpqp3k
or any sheet with more > formulas than will fit on screen. > 2. Swipe to scroll formula > > Results: > Formula region does not scroll.
Following these original reproduction steps, I think we can see that Quirk 2 is no longer necessary by observing that is not applied to
https://www.desmos.com/calculator
but is applied to saved graph URLs such as
https://www.desmos.com/calculator/utywo6ohyn
. You can try 1. visiting
https://www.desmos.com/calculator
on an iPad running iPadOS 15 or 16 (and likely other versions), where the quirk is not applied 2. Entering many formulas, say 20 formulas with the first 20 integers 3. Observing that swiping to scroll the list of formulas works correctly vs. 1. visiting
https://www.desmos.com/calculator/utywo6ohyn
(a saved graph where the quirk is applied) 2. Observing that the list of formulas is also scrollable by swiping here.
Karl Dubost
Comment 6
2022-11-09 20:19:26 PST
Pull request:
https://github.com/WebKit/WebKit/pull/6332
EWS
Comment 7
2022-11-10 17:37:24 PST
Committed
256558@main
(8e0e88f4f3d2): <
https://commits.webkit.org/256558@main
> Reviewed commits have been landed. Closing PR #6332 and removing active labels.
Karl Dubost
Comment 8
2022-11-13 19:23:13 PST
Jason, One of the Quirks has been deactivated on WebKit.
Jason Merrill
Comment 9
2022-11-14 09:33:07 PST
That's great. Thank you.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug