Bug 58656

Created attachment 89813 [details] Patch Fix bug. Now QtTestBrowser has the same behavior of chrome and firefox.

Comment on attachment 89813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89813&action=review r- because of the empty ChangeLog. > Source/WebKit/qt/Api/qwebpage.cpp:750 > - focusFrame->editor()->copy(); > + Pasteboard::generalPasteboard()->writeSelection(focusFrame->editor()->selectedRange().get(), focusFrame->editor()->canSmartCopyOrDelete(), focusFrame); Editor::copy() will make a text-only copy if the selection is inside a text form control. After this change, we always make multi-part copies (with rich-text metadata.) Does this match the behavior of Chromium on Linux? I.e do copies made using X11 selection contain rich-text data? > Source/WebKit/qt/ChangeLog:6 > + [Qt] Text selection is causing oncopy event be called > + https://bugs.webkit.org/show_bug.cgi?id=58656 We need more explanation of what's being changed and why in the ChangeLog.

Created attachment 89983 [details] Patch updating patch changelog

Comment on attachment 89983 [details] Patch No tests?

Created attachment 90021 [details] Patch Adding test

Comment on attachment 90021 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90021&action=review > LayoutTests/editing/pasteboard/text-selection-expected.txt:2 > +This file tests if text selection is causing oncopy event to be called. This test requires DRT. > +oncopy event called While this output is correct AFAICT, it certainly looks wrong. Please format the output in a way that makes it clear what is being tested and what the expected results are. > Source/WebKit/qt/ChangeLog:13 > + Always when a text is selected the oncopy event is called, this behavior is > + not presented by browsers such as firefox and chrome. Now, when selecting > + a text, QtWebkit is making multi part-copies (with rich text metadata), the multi-part > + data can be obtained by data transfer interface. > + Also, Copies to the clipboard of a selected image, is not supported by browsers > + like chrome and firefox and was removed from QtWebkit. s/browsers such as// s/firefox/Firefox/ s/chrome/Chrome/ s/a text/text/ s/QtWebkit/QtWebKit/ Can we actually obtain the multi-part selection via the data transfer interface in QtWebKit today?

Created attachment 90025 [details] Patch Changelog fixes and updating test to check how many times the oncopy method is called.

> Can we actually obtain the multi-part selection via the data transfer interface in QtWebKit today? Right now it is not supported so in changelog i put "... when supported by QtWebKit".

Comment on attachment 90025 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90025&action=review > Source/WebKit/qt/ChangeLog:9 > + not presented by Firefox and Chrome. Now, when selecting a text, QtWebKit , this behavior is not present in Firefox or Chrome. > Source/WebKit/qt/ChangeLog:11 > + data can be obtained by data transfer interface when supported by QtWebKit. when it will be supported by QtWebKit. > Source/WebKit/qt/ChangeLog:12 > + Also, Copies to the clipboard of a selected image, is not supported by Chrome and Copies -> copies

(In reply to comment #9) > (From update of attachment 90025 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90025&action=review > > > Source/WebKit/qt/ChangeLog:9 > > + not presented by Firefox and Chrome. Now, when selecting a text, QtWebKit > > , this behavior is not present in Firefox or Chrome. or this behavior does not exist in Firefox or Chrome. > > > Source/WebKit/qt/ChangeLog:11 > > + data can be obtained by data transfer interface when supported by QtWebKit. > > when it will be supported by QtWebKit. > > > Source/WebKit/qt/ChangeLog:12 > > + Also, Copies to the clipboard of a selected image, is not supported by Chrome and > > Copies -> copies

Comment on attachment 90025 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90025&action=review > LayoutTests/editing/pasteboard/text-selection-expected.txt:2 > +This file tests if text selection is causing oncopy event to be called. This test requires DRT. > +oncopy event called 1 time Please explain that the text below should read that the oncopy event was triggered only once. The output could be something like: This file tests if text selection causes oncopy events. This test requires DRT. Selecting text with mouse, no copy events should occur: Event count: 0 Copying text, 1 copy event should occur: Event count: 1 > LayoutTests/editing/pasteboard/text-selection.html:37 > + eventSender.mouseMoveTo(0, 0); > + for (var i = 0; i < 3; i++) { > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } This is making a triple-click to select the whole line? Will it work on other platforms? This test will be run on the Mac, Chromium, Gtk ports, etc. I don't know if they all support triple-click selection. It would be cleaner to do a normal selection via mouseDown, mouseMove, mouseUp over the text.

Created attachment 90072 [details] Patch Updating test .

Created attachment 90077 [details] Patch Some aesthetics fixes

Comment on attachment 90077 [details] Patch LGTM, fingers crossed that the test works everywhere :)

Comment on attachment 90077 [details] Patch Rejecting attachment 90077 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 1 Last 500 characters of output: autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain result = func(*args) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open return self.do_open(conn_factory, req) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open raise URLError(err) urllib2.URLError: <urlopen error [Errno 60] Operation timed out> Full output: http://queues.webkit.org/results/8468190

Comment on attachment 90077 [details] Patch Let's try again

Comment on attachment 90077 [details] Patch Rejecting attachment 90077 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 1 Last 500 characters of output: autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain result = func(*args) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open return self.do_open(conn_factory, req) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open raise URLError(err) urllib2.URLError: <urlopen error [Errno 60] Operation timed out> Full output: http://queues.webkit.org/results/8470333

Created attachment 90208 [details] Patch updating patch

The commit-queue encountered the following flaky tests while processing attachment 90208 [details]: http/tests/xmlhttprequest/cross-origin-no-authorization.html bug 33357 (author: ap@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 90208 [details] Patch Clearing flags on attachment: 90208 Committed r84263: <http://trac.webkit.org/changeset/84263>

All reviewed patches have been landed. Closing bug.