rdar://78330664
Created attachment 430603 [details] Patch
Created attachment 430607 [details] Patch
Comment on attachment 430607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430607&action=review > Source/WebKit/WebProcess/WebPage/WebPage.h:2195 > bool m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick { false }; > + bool m_didChangePlayingMediaStateDuringSyntheticClick { false }; How many of these are we going to end up with? Is it actually important that we keep track of /why/? Should we just "m_currentSyntheticClickMayBeMeaningful { true }" and then falsy it in all the callbacks? Then you don't need m_isCompletingSyntheticClick or separate bits for each thing.
(In reply to Tim Horton from comment #3) > Comment on attachment 430607 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430607&action=review > > > Source/WebKit/WebProcess/WebPage/WebPage.h:2195 > > bool m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick { false }; > > + bool m_didChangePlayingMediaStateDuringSyntheticClick { false }; > > How many of these are we going to end up with? Is it actually important that > we keep track of /why/? > Should we just "m_currentSyntheticClickMayBeMeaningful { true }" and then > falsy it in all the callbacks? Then you don't need > m_isCompletingSyntheticClick or separate bits for each thing. That's a good point — I'll change it to `m_currentSyntheticClickMayBeMeaningful` instead, in the way you described. (As an aside, if we accumulate one or two more of these call sites in the future, we'll also probably want this to also turn it into an OptionSet).
Created attachment 430611 [details] Patch
Comment on attachment 430611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430611&action=review r=me as well :) > Source/WebKit/WebProcess/WebPage/WebPage.h:2193 > + bool m_currentSyntheticClickMayBeNonMeaningful { true }; NIT: How about `m_currentSyntheticClickMayNotBeMeaningful`? 😅
(In reply to Devin Rousso from comment #6) > Comment on attachment 430611 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430611&action=review > > r=me as well :) > > > Source/WebKit/WebProcess/WebPage/WebPage.h:2193 > > + bool m_currentSyntheticClickMayBeNonMeaningful { true }; > > NIT: How about `m_currentSyntheticClickMayNotBeMeaningful`? 😅 Sure — I don't have a preference for either one, so I'll change it to `m_currentSyntheticClickMayNotBeMeaningful`.
Created attachment 430616 [details] Renaming
Committed r278515 (238514@main): <https://commits.webkit.org/238514@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430616 [details].