The WebCore::EventHandler::targetPositionInWindowForSelectionAutoscroll method is retrieving NSScreen information directly. These calls should be brokered.
<rdar://problem/39224969>
Created attachment 337403 [details] Patch
Comment on attachment 337403 [details] Patch Attachment 337403 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7231943 New failing tests: fast/events/autoscroll-with-non-scrollable-parent.html fast/events/autoscroll-nonscrollable-iframe-in-scrollable-div.html fast/events/no-scroll-on-input-text-selection.html fast/forms/select-multiple-elements-with-mouse-drag.html fast/events/autoscroll-in-iframe.html fast/forms/input-readonly-autoscroll.html fast/events/autoscroll-in-textfield.html fast/events/autoscroll-should-not-stop-on-keypress.html fast/forms/select/listbox-disabled-no-autoscroll.html fast/events/autoscroll-in-iframe-body.html
Created attachment 337408 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 337403 [details] Patch Attachment 337403 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7232000 New failing tests: fast/events/autoscroll-with-non-scrollable-parent.html fast/events/autoscroll-nonscrollable-iframe-in-scrollable-div.html fast/events/no-scroll-on-input-text-selection.html fast/forms/select-multiple-elements-with-mouse-drag.html fast/events/autoscroll-in-iframe.html fast/forms/input-readonly-autoscroll.html fast/events/autoscroll-in-textfield.html fast/events/autoscroll-should-not-stop-on-keypress.html fast/forms/select/listbox-disabled-no-autoscroll.html fast/events/autoscroll-in-iframe-body.html
Created attachment 337409 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 337430 [details] Patch
Created attachment 337515 [details] Patch
Created attachment 337521 [details] Patch
Comment on attachment 337515 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337515&action=review > Source/WebCore/page/mac/EventHandlerMac.mm:1169 > + auto frame = toUserSpaceForPrimaryScreen(screenProperties(page->chrome().displayID()).screenRect); Will the screen properties map be empty on WK1?
(In reply to Per Arne Vollan from comment #10) > Comment on attachment 337515 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=337515&action=review > > > Source/WebCore/page/mac/EventHandlerMac.mm:1169 > > + auto frame = toUserSpaceForPrimaryScreen(screenProperties(page->chrome().displayID()).screenRect); > > Will the screen properties map be empty on WK1? Oh, you are right. This will crash in WebKitLegacy. :-(
Created attachment 337529 [details] Patch
Created attachment 337535 [details] Patch
Comment on attachment 337535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337535&action=review > Source/WebCore/page/mac/EventHandlerMac.mm:-1167 > - auto frame = toUserSpace(screen(page->chrome().displayID()).frame, nil); It seems the original transformation will be incorrect if the window is not on the first screen, since the NSWindow parameter is nil here. Should we also address that in this patch?
(In reply to Per Arne Vollan from comment #14) > Comment on attachment 337535 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=337535&action=review > > > Source/WebCore/page/mac/EventHandlerMac.mm:-1167 > > - auto frame = toUserSpace(screen(page->chrome().displayID()).frame, nil); > > It seems the original transformation will be incorrect if the window is not > on the first screen, since the NSWindow parameter is nil here. Should we > also address that in this patch? Which NSWindow would we be passing as the second argument? If you know the right answer one to use, I’m in favor of fixing it now.
Comment on attachment 337535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337535&action=review >>> Source/WebCore/page/mac/EventHandlerMac.mm:-1167 >>> - auto frame = toUserSpace(screen(page->chrome().displayID()).frame, nil); >> >> It seems the original transformation will be incorrect if the window is not on the first screen, since the NSWindow parameter is nil here. Should we also address that in this patch? > > Which NSWindow would we be passing as the second argument? If you know the right answer one to use, I’m in favor of fixing it now. Actually, I think it is correct as-is. A nil window was used by the other eiginslncode to compute coordinates with respect to the primary display (I.e., not “no display”). I think that is the correct transformation.
Comment on attachment 337535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337535&action=review Very nice! r=me. > Source/WebKit/WebProcess/WebProcess.cpp:138 > +#if PLATFORM(MAC) > +#include <WebCore/PlatformScreen.h> > +#endif This include might not be needed.
Comment on attachment 337535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337535&action=review >> Source/WebKit/WebProcess/WebProcess.cpp:138 >> +#endif > > This include might not be needed. You are right. I'll remove it.
Committed r230454: <https://trac.webkit.org/changeset/230454>