Expand r209943 to suppress paste during provisional navigation as well

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Expand r209943 to suppress paste during provisional navigation as well

[Tim Horton]

Created attachment 324101 [details]
Patch

[Sam Weinig]

Comment on attachment 324101 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324101&action=review

> Source/WebCore/editing/Editor.cpp:459
> +    if (m_frame.mainFrame().loader().shouldSuppressKeyboardInput())
> +        return false;

Given that paste can be triggered from something that isn't the keyboard, this seems like a weird check here.  That probably implies shouldSuppressKeyboardInput() needs a better name to indicate what it really wants.

[Sam Weinig]

Comment on attachment 324101 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324101&action=review

>> Source/WebCore/editing/Editor.cpp:459
>> +        return false;
> 
> Given that paste can be triggered from something that isn't the keyboard, this seems like a weird check here.  That probably implies shouldSuppressKeyboardInput() needs a better name to indicate what it really wants.

Also, I think canPaste() gets called after we dispatch the paste events.  Perhaps a better place for this would be in EditorCommands.cpp in enabledPaste?

[Sam Weinig]

(In reply to Sam Weinig from comment #3)
> Also, I think canPaste() gets called after we dispatch the paste events. 
> Perhaps a better place for this would be in EditorCommands.cpp in
> enabledPaste?

Ha, enabledPaste just calls canPaste() :).  So it looks like it probably does block the paste events (but perhaps you should add a test for it).

[Tim Horton]

(In reply to Sam Weinig from comment #2)
> Comment on attachment 324101 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324101&action=review
> 
> > Source/WebCore/editing/Editor.cpp:459
> > +    if (m_frame.mainFrame().loader().shouldSuppressKeyboardInput())
> > +        return false;
> 
> Given that paste can be triggered from something that isn't the keyboard,
> this seems like a weird check here.  That probably implies
> shouldSuppressKeyboardInput() needs a better name to indicate what it really
> wants.

Yes, that is true.

(In reply to Sam Weinig from comment #4)
> Ha, enabledPaste just calls canPaste() :).  So it looks like it probably
> does block the paste events (but perhaps you should add a test for it).

I'll add that to the existing test.

[Tim Horton]

Created attachment 324181 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 324181 [details]
Patch

Clearing flags on attachment: 324181

Committed r223708: <https://trac.webkit.org/changeset/223708>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35568708>