Previews should ignore all mouse events except scrolling.
<rdar://problem/18876437>
Created attachment 240996 [details] Patch
Comment on attachment 240996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240996&action=review > Source/WebKit2/UIProcess/API/mac/WKView.mm:1211 > - if ([self shouldIgnoreMouseEvents]) > + if ([self _shouldIgnoreWheelEvents]) What does this do if _ignoreNonWheelMouseEvents returns false but shouldIgnoreMouseEvents returns true?
(In reply to comment #3) > Comment on attachment 240996 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240996&action=review > > > Source/WebKit2/UIProcess/API/mac/WKView.mm:1211 > > - if ([self shouldIgnoreMouseEvents]) > > + if ([self _shouldIgnoreWheelEvents]) > > What does this do if _ignoreNonWheelMouseEvents returns false but > shouldIgnoreMouseEvents returns true? -_shouldIgnoreWheelEvents contains the same logic that used to be in -shouldIgnoreMouseEvents. In the new implementation, the only way the above condition can occur is if have a thumbnail view, in which case -_shouldIgnoreWheelEvents will also return true.
(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 240996 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=240996&action=review > > > > > Source/WebKit2/UIProcess/API/mac/WKView.mm:1211 > > > - if ([self shouldIgnoreMouseEvents]) > > > + if ([self _shouldIgnoreWheelEvents]) > > > > What does this do if _ignoreNonWheelMouseEvents returns false but > > shouldIgnoreMouseEvents returns true? > > -_shouldIgnoreWheelEvents contains the same logic that used to be in > -shouldIgnoreMouseEvents. > > In the new implementation, the only way the above condition can occur is if > have a thumbnail view, in which case -_shouldIgnoreWheelEvents will also > return true. Does that mean that an existing WKView subclass who overrides shouldIgnoreMouseEvents and returns true will stop working?
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > Comment on attachment 240996 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=240996&action=review > > > > > > > Source/WebKit2/UIProcess/API/mac/WKView.mm:1211 > > > > - if ([self shouldIgnoreMouseEvents]) > > > > + if ([self _shouldIgnoreWheelEvents]) > > > > > > What does this do if _ignoreNonWheelMouseEvents returns false but > > > shouldIgnoreMouseEvents returns true? > > > > -_shouldIgnoreWheelEvents contains the same logic that used to be in > > -shouldIgnoreMouseEvents. > > > > In the new implementation, the only way the above condition can occur is if > > have a thumbnail view, in which case -_shouldIgnoreWheelEvents will also > > return true. > > Does that mean that an existing WKView subclass who overrides > shouldIgnoreMouseEvents and returns true will stop working? In spite of the non-prefixed method name, -shouldIgnoreMouseEvents is purely internal and shouldn't be overridden by subclasses.
Comment on attachment 240996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240996&action=review Patch looks good other than the declaration being in WKWebViewPrivate instead of WKWebViewinternal. > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:157 > +@property (nonatomic, setter=_setIgnoresNonWheelMouseEvents:) BOOL _ignoresNonWheelMouseEvents; If this is only used internally, it should go into WKWebViewInternal instead. >>>>> Source/WebKit2/UIProcess/API/mac/WKView.mm:1211 >>>>> + if ([self _shouldIgnoreWheelEvents]) >>>> >>>> What does this do if _ignoreNonWheelMouseEvents returns false but shouldIgnoreMouseEvents returns true? >>> >>> -_shouldIgnoreWheelEvents contains the same logic that used to be in -shouldIgnoreMouseEvents. >>> >>> In the new implementation, the only way the above condition can occur is if have a thumbnail view, in which case -_shouldIgnoreWheelEvents will also return true. >> >> Does that mean that an existing WKView subclass who overrides shouldIgnoreMouseEvents and returns true will stop working? > > In spite of the non-prefixed method name, -shouldIgnoreMouseEvents is purely internal and shouldn't be overridden by subclasses. Fair enough, let's hope nobody replies on it.
(In reply to comment #7) > Comment on attachment 240996 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240996&action=review > > Patch looks good other than the declaration being in WKWebViewPrivate > instead of WKWebViewinternal. > > > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:157 > > +@property (nonatomic, setter=_setIgnoresNonWheelMouseEvents:) BOOL _ignoresNonWheelMouseEvents; > > If this is only used internally, it should go into WKWebViewInternal instead. Thanks, will post a new patch that fixes this.
Created attachment 241040 [details] Patch
Comment on attachment 241040 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241040&action=review > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:103 > + [webView _setIgnoresNonWheelMouseEvents:YES]; This should use property syntax.
(In reply to comment #10) > Comment on attachment 241040 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241040&action=review > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:103 > > + [webView _setIgnoresNonWheelMouseEvents:YES]; > > This should use property syntax. That should actually be using a RetainPtr; will post a patch that addresses this.
Created attachment 241043 [details] Patch
Comment on attachment 241043 [details] Patch Clearing flags on attachment: 241043 Committed r175630: <http://trac.webkit.org/changeset/175630>
All reviewed patches have been landed. Closing bug.