RESOLVED FIXED 221948
REGRESSION (r266695): Unable to scroll the menu in 北京114预约挂号 official account - WeChat
https://bugs.webkit.org/show_bug.cgi?id=221948
Summary REGRESSION (r266695): Unable to scroll the menu in 北京114预约挂号 official account...
Myles C. Maxfield
Reported 2021-02-15 23:09:21 PST
REGRESSION (r266695): Unable to scroll the menu in 北京114预约挂号 official account - WeChat
Attachments
Patch (6.22 KB, patch)
2021-02-15 23:13 PST, Myles C. Maxfield
no flags
Patch (6.82 KB, patch)
2021-02-16 17:51 PST, Myles C. Maxfield
no flags
Patch (6.83 KB, patch)
2021-02-16 17:56 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Patch (6.91 KB, patch)
2021-02-16 18:14 PST, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2021-02-15 23:13:35 PST
Myles C. Maxfield
Comment 2 2021-02-15 23:14:20 PST
Sergio Villar Senin
Comment 3 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()
Simon Fraser (smfr)
Comment 4 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.
Simon Fraser (smfr)
Comment 5 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.
Myles C. Maxfield
Comment 6 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?
Simon Fraser (smfr)
Comment 7 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?
Myles C. Maxfield
Comment 8 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.
Myles C. Maxfield
Comment 9 2021-02-16 17:51:59 PST
Myles C. Maxfield
Comment 10 2021-02-16 17:56:31 PST
Myles C. Maxfield
Comment 11 2021-02-16 18:14:43 PST
EWS
Comment 12 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].
Note You need to log in before you can comment on or make changes to this bug.