Fix selection handling in pasteboard test
Created attachment 191462 [details] Patch
Attachment 191462 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/editing/pasteboard/8145-2.html', u'LayoutTests/platform/chromium-win/editing/pasteboard/8145-2-expected.txt', u'LayoutTests/platform/chromium/editing/pasteboard/8145-2-expected.txt', u'LayoutTests/platform/efl/editing/pasteboard/8145-2-expected.txt', u'LayoutTests/platform/gtk/editing/pasteboard/8145-2-expected.txt', u'LayoutTests/platform/mac/editing/pasteboard/8145-2-expected.txt', u'LayoutTests/platform/qt/editing/pasteboard/8145-2-expected.txt']" exit_code: 1 LayoutTests/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 191466 [details] Patch
Created attachment 191476 [details] Patch
Comment on attachment 191476 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191476&action=review > LayoutTests/ChangeLog:11 > + for this it's been using move by word and move by character. The > + outcome of this might depend on the platform, so use move by > + paragraph instead. Do we have a way to force specific platform behavior in the test? Many editing behaviors are overridable via internals. It is better to have a way to test all platform behaviors in regression tests, not to avoid things that are platform dependent.
Comment on attachment 191476 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191476&action=review >> LayoutTests/ChangeLog:11 >> + paragraph instead. > > Do we have a way to force specific platform behavior in the test? Many editing behaviors are overridable via internals. > > It is better to have a way to test all platform behaviors in regression tests, not to avoid things that are platform dependent. I agree with Alexey. r-.
(In reply to comment #5) > (From update of attachment 191476 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=191476&action=review > > > LayoutTests/ChangeLog:11 > > + for this it's been using move by word and move by character. The > > + outcome of this might depend on the platform, so use move by > > + paragraph instead. > > Do we have a way to force specific platform behavior in the test? Many editing behaviors are overridable via internals. Yes, it is possible. > It is better to have a way to test all platform behaviors in regression tests, not to avoid things that are platform dependent. I agree with you, just that in this case what it's been tested is not how the selection is changed but what happens afterwards. There are other tests that cover selection manipulation.
> I agree with you, just that in this case what it's been tested is not how the selection is changed but what happens afterwards. Yes, it's true that this is not what is being tested here. But wouldn't it be better to make a less invasive change anyway? Additionally, moving by paragraph is an even less established operation than moving by word, so this patch just makes the test to rely on another assumption.
(In reply to comment #8) > > I agree with you, just that in this case what it's been tested is not how the selection is changed but what happens afterwards. > > Yes, it's true that this is not what is being tested here. But wouldn't it be better to make a less invasive change anyway? > > Additionally, moving by paragraph is an even less established operation than moving by word, so this patch just makes the test to rely on another assumption. I see. So, if I understand this correctly, instead of fixing this here what I should do is to just force a non-Windows behavior in these tests when landing bug 110487. Is that correct?
Yes, this sounds right to me.
(In reply to comment #10) > Yes, this sounds right to me. Thanks for your input. I'll mark this as WONTFIX then.