<https://mail.yahoo.com/> serves the mobile site even in desktop browsing "mode"
<rdar://problem/59824469>
Created attachment 406317 [details] Patch
Comment on attachment 406317 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406317&action=review > Source/WebCore/page/Quirks.cpp:501 > + auto& url = m_document->topDocument().url(); > + auto host = url.host().toString(); > + if (host.startsWithIgnoringASCIICase("mail.") && topPrivatelyControlledDomain(host).startsWith("yahoo.")) > + return true; > + > + return false; Can do this more efficiently like so: auto host = m_document->topDocument().url().host(); return startsWithLettersIgnoringASCIICase(host, "mail.") && topPrivatelyControlledDomain(host.toString()).startsWith("yahoo."); Also, I think we should write an isYahooMail helper function so we can share this logic with Quirks::shouldAvoidPastingImagesAsWebContent. Also, that uses a data member to cache the result. Should we be doing that here too? Or sharing a single one?
Comment on attachment 406317 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406317&action=review > Source/WebCore/ChangeLog:12 > + <https://mail.yahoo.com/> serves the mobile site even in desktop browsing "mode". There's a bit of missing context here: that the mobile site doesn't work well with mouse events in a particular case. > Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:570 > +#if ENABLE(IOS_TOUCH_EVENTS) > + if (mouseEventPolicy == MouseEventPolicy::Default && document->quirks().shouldSynthesizeTouchEvents()) > + mouseEventPolicy = MouseEventPolicy::SynthesizeTouchEvents; > +#endif Is there a reason we can't tuck this away in the DocumentLoader mouseEventPolicy() getter instead?
Comment on attachment 406317 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406317&action=review >> Source/WebCore/page/Quirks.cpp:501 >> + return false; > > Can do this more efficiently like so: > > auto host = m_document->topDocument().url().host(); > return startsWithLettersIgnoringASCIICase(host, "mail.") && topPrivatelyControlledDomain(host.toString()).startsWith("yahoo."); > > Also, I think we should write an isYahooMail helper function so we can share this logic with Quirks::shouldAvoidPastingImagesAsWebContent. Also, that uses a data member to cache the result. Should we be doing that here too? Or sharing a single one? A shared function is a good idea I'll do that. As far as saving this to a variable, I was thinking that this is only called during top-level/frame navigations so it wouldn't happen that often, but perhaps caching it is a good idea. >> Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:570 >> +#endif > > Is there a reason we can't tuck this away in the DocumentLoader mouseEventPolicy() getter instead? I was trying to follow what `DocumentLoader::simulatedMouseEventsDispatchPolicy` does, but I think your idea is probably cleaner. I'll change it.
Created attachment 406342 [details] Patch
Created attachment 406351 [details] Patch fix win build
Committed r265485: <https://trac.webkit.org/changeset/265485> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406351 [details].