Bug 221948 - REGRESSION (r266695): Unable to scroll the menu in 北京114预约挂号 official account - WeChat
Summary: REGRESSION (r266695): Unable to scroll the menu in 北京114预约挂号 official account...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-15 23:09 PST by Myles C. Maxfield
Modified: 2021-02-16 20:10 PST (History)
6 users (show)

See Also:


Attachments
Patch (6.22 KB, patch)
2021-02-15 23:13 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (6.82 KB, patch)
2021-02-16 17:51 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (6.83 KB, patch)
2021-02-16 17:56 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (6.91 KB, patch)
2021-02-16 18:14 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-02-15 23:09:21 PST
REGRESSION (r266695): Unable to scroll the menu in 北京114预约挂号 official account - WeChat
Comment 1 Myles C. Maxfield 2021-02-15 23:13:35 PST
Created attachment 420425 [details]
Patch
Comment 2 Myles C. Maxfield 2021-02-15 23:14:20 PST
<rdar://problem/71661277>
Comment 3 Sergio Villar Senin 2021-02-16 06:39:13 PST
Comment on attachment 420425 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420425&action=review

Discussed this in private with Myles. Looks good to me, but I'd let Apple folks to give the final r+.

> Source/WebCore/style/StyleAdjuster.cpp:610
> +    if (m_document.quirks().needsWeChatScrollingQuirk()) {

Would it make sense to wrap it in #if PLATFORM(IOS_FAMILY) ?

That would imply also wrapping needsWeChatScrollingQuirk() isWeChat()
Comment 4 Simon Fraser (smfr) 2021-02-16 08:56:43 PST
Comment on attachment 420425 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420425&action=review

> Source/WebCore/style/StyleAdjuster.cpp:612
> +        static MainThreadNeverDestroyed<const AtomString> attribute1("data-v-f9dfb3e2", AtomString::ConstructFromLiteral);
> +        static MainThreadNeverDestroyed<const AtomString> attribute2("data-v-33719130", AtomString::ConstructFromLiteral);

It seems very likely that they could re-deploy content and these identifiers would change.
Comment 5 Simon Fraser (smfr) 2021-02-16 09:39:00 PST
Comment on attachment 420425 [details]
Patch

Also I think we should disable this with a linked-on-or-faster check.
Comment 6 Myles C. Maxfield 2021-02-16 16:27:14 PST
(In reply to Simon Fraser (smfr) from comment #4)
> Comment on attachment 420425 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420425&action=review
> 
> > Source/WebCore/style/StyleAdjuster.cpp:612
> > +        static MainThreadNeverDestroyed<const AtomString> attribute1("data-v-f9dfb3e2", AtomString::ConstructFromLiteral);
> > +        static MainThreadNeverDestroyed<const AtomString> attribute2("data-v-33719130", AtomString::ConstructFromLiteral);
> 
> It seems very likely that they could re-deploy content and these identifiers
> would change.

I was trying to be as specific as possible, because whatever condition is here will be applied across all of WeChat (including all of its mini-apps and everything). Do you think I was overly conservative? Do you have a suggestion for a better way to isolate this case without being too general?
Comment 7 Simon Fraser (smfr) 2021-02-16 16:44:06 PST
Are there other nodes that match the style and "tree-select" and "v-tree-select" checks that don't want the fix? Presumably if this particular page is broken, there may be other pages that are also broken, so a slightly more permissible check might better?
Comment 8 Myles C. Maxfield 2021-02-16 17:18:41 PST
Comment on attachment 420425 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420425&action=review

>> Source/WebCore/style/StyleAdjuster.cpp:610
>> +    if (m_document.quirks().needsWeChatScrollingQuirk()) {
> 
> Would it make sense to wrap it in #if PLATFORM(IOS_FAMILY) ?
> 
> That would imply also wrapping needsWeChatScrollingQuirk() isWeChat()

I think not, because needsWeChatScrollingQuirk() already has the check inside it.
Comment 9 Myles C. Maxfield 2021-02-16 17:51:59 PST
Created attachment 420568 [details]
Patch
Comment 10 Myles C. Maxfield 2021-02-16 17:56:31 PST
Created attachment 420572 [details]
Patch
Comment 11 Myles C. Maxfield 2021-02-16 18:14:43 PST
Created attachment 420575 [details]
Patch
Comment 12 EWS 2021-02-16 20:10:19 PST
Committed r272977: <https://commits.webkit.org/r272977>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420575 [details].