Device name: Samsung Galaxy S3 OS: ICS 4.0.4 Chrome: 18.0.1025308 When a webpage has a text field (input field) that is disabled, tap and hold on such field does not open clipboard functions menu to select/copy text from that field. Neither text selection nor copy to clipboard entire text is possible.
Created attachment 169369 [details] Patch
Comment on attachment 169369 [details] Patch Attachment 169369 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14395946 New failing tests: fast/events/touch/gesture/disabled-input-text-selection.html
Comment on attachment 169369 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169369&action=review > Source/WebCore/page/EventHandler.cpp:2579 > - if (!result.isLiveLink() && innerNode && (innerNode->isContentEditable() || innerNode->isTextNode())) { > + if (!result.isLiveLink() && innerNode && innerNode->canStartSelection()) { It appears that sendContextMenuEvent has the same issue. > LayoutTests/fast/events/touch/gesture/disabled-input-text-selection.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> Please use HTML5 style DOCTYPE: <!DOCTYPE html> > LayoutTests/fast/events/touch/gesture/disabled-input-text-selection.html:9 > +<div id="result">FAIL</div> If you used shouldBe, then you wouldn't need this. > LayoutTests/fast/events/touch/gesture/disabled-input-text-selection.html:26 > + if (eventSender.gestureLongPress) { > + eventSender.gestureLongPress(x, y); > + } else { No curly brackets around a single line statement. > LayoutTests/fast/events/touch/gesture/disabled-input-text-selection.html:32 > + if (window.getSelection() == "Disabled") > + document.getElementById("result").innerHTML = "PASS"; Why don't you use shouldBe() instead?
This is not specific to Chromium.
Comment on attachment 169369 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169369&action=review > Source/WebCore/ChangeLog:3 > + Disabled input/textarea doesn't trigger selection change I just renamed the bug title. Please update change logs to reflect that when you upload a new patch. > LayoutTests/ChangeLog:3 > + Disabled input/textarea doesn't trigger selection change Ditto.
Created attachment 169763 [details] Patch
Comment on attachment 169763 [details] Patch Attachment 169763 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14456840 New failing tests: fast/events/touch/gesture/disabled-input-text-selection.html
Created attachment 169839 [details] Patch
Comment on attachment 169839 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169839&action=review > LayoutTests/platform/chromium/TestExpectations:3143 > +webkit.org/b/99698 fast/events/touch/gesture/disabled-input-text-selection.html [ Failure Pass ] Why is this marked as flaky?
(In reply to comment #9) > (From update of attachment 169839 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169839&action=review > > > LayoutTests/platform/chromium/TestExpectations:3143 > > +webkit.org/b/99698 fast/events/touch/gesture/disabled-input-text-selection.html [ Failure Pass ] > > Why is this marked as flaky? Because Long touch gesture selecting word is not a valid test case for chromium port. I am thinking, that long press should be passed as double click in case of chromium port. Correct me if wrong.
Created attachment 170084 [details] Patch
Comment on attachment 170084 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170084&action=review > Source/WebCore/page/EventHandler.cpp:2579 > - if (!result.isLiveLink() && innerNode && (innerNode->isContentEditable() || innerNode->isTextNode())) { > + if (!result.isLiveLink() && innerNode && innerNode->canStartSelection()) { Why don't you use canMouseDownStartSelect here? Also, I said, EventHandler::sendContextMenuEvent has a similar code. Why is it okay not to change that? > LayoutTests/fast/events/touch/gesture/disabled-input-text-selection.html:21 > +if (!window.eventSender) > + return; return?? It doesn't do anything useful here. > LayoutTests/fast/events/touch/gesture/disabled-input-text-selection.html:27 > + return; Ditto.
Created attachment 171410 [details] Patch
Comment on attachment 171410 [details] Patch Attachment 171410 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14640468 New failing tests: editing/selection/5354455-2.html
Comment on attachment 171410 [details] Patch Clearing flags on attachment: 171410 Committed r133416: <http://trac.webkit.org/changeset/133416>
All reviewed patches have been landed. Closing bug.
Comment on attachment 171410 [details] Patch Trying again. Seems to be tree was red.
This seems to have regressed editing/selection/5354455-2.html on Mac: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=editing%2Fselection%2F5354455-2.html If I'm reading the test correctly, my suspicion is that a right-click outside of a text element no longer cancels the selection.
(In reply to comment #18) > This seems to have regressed editing/selection/5354455-2.html on Mac: > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=editing%2Fselection%2F5354455-2.html > > If I'm reading the test correctly, my suspicion is that a right-click outside of a text element no longer cancels the selection. OK, I tested this manually (http://trac.webkit.org/export/133422/trunk/LayoutTests/editing/selection/5354455-2.html) with a ToT build, and it seems that a right-click outside the box (text area) now causes the first word in the paragraph to be selected. So I think this is a real bug.
Also causing a failure in browser_tests RenderViewImplTest.ContextMenu: http://build.chromium.org/p/chromium.webkit/builders/Mac10.6%20Tests/builds/14387
Comment on attachment 171410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171410&action=review > Source/WebCore/page/EventHandler.cpp:2586 > - if (!result.isLiveLink() && innerNode && (innerNode->isContentEditable() || innerNode->isTextNode())) { > + if (!result.isLiveLink() && innerNode && canMouseDownStartSelect(innerNode)) { Ah, we're now missing innerNode->isTextNode() check. > Source/WebCore/page/EventHandler.cpp:2703 > - && (m_frame->selection()->isContentEditable() || (mev.targetNode() && mev.targetNode()->isTextNode()))) { > + && canMouseDownStartSelect(mev.targetNode())) { Ditto.
Comment on attachment 171410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171410&action=review >> Source/WebCore/page/EventHandler.cpp:2703 >> + && canMouseDownStartSelect(mev.targetNode())) { > > Ditto. Or rather, this line shouldn't have been changed :(
The correct fix isn't obvious. We need to roll out this patch.
Re-opened since this is blocked by bug 101169
Here's a problem. The bug says we need to select the closest word even when hit test target isn't an editable or text node. However, this behavior isn't desirable outside of an input field. Perhaps what we want to do is to special case textarea and input elements. But that won't work for any "component" implemented by shadow DOM API. Perhaps we want to do this only if we're on the right of some text (in English. which means on the right of RTL text, and on the bottom of vertically written top-down text or on the top of vertically written bottom-up text).
a
As far i see, the issue is only related to Chromium on android. So keeping Context menu conditions in tact will be better. I will just need to change the condition for Android case leaving other context menu condition as it is.
(In reply to comment #27) > As far i see, the issue is only related to Chromium on android. So keeping Context menu conditions in tact will be better. I will just need to change the condition for Android case leaving other context menu condition as it is. No, that won’t work. Your patch will enable long press anywhere on the page and will select the wrong word. Open editing/selection/5354455-2.html and try it yourself.
bassza00133@hotmail.com
Created attachment 469383 [details] bassza00133@gmail.com https://developers.google.com/privacy-sandbox/3pcd/chips?hl=th#cookie_partitioning_technical_design bassza00133@gmail.com
bassza00133@gmail.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/metadata/contributors.json. Rejecting attachment 469383 [details] from commit queue.