Issue metadata
Sign in to add a comment
|
SelectionPopup menu display error
Reported by
fanjins...@sogou-inc.com,
Dec 19
|
||||||||||||||||||||||
Issue descriptionSteps to reproduce the problem: 1. Sync the latest chromium source codes for Android and 'ninja -C out/Release webview_instrumentation_apk' 2. Install WebViewInstrumentation.apk and load https://m.sohu.com 3. Open any news, and select text as shown in Figure 1 4. Click editable box '我来说两句' 5. Long press it, then popup Paste menu, click 'paste', and clear content in the editable box 6. Long press the editable box again, then popup 'Paste | Select All' menu shown as Figure 2 7. Click 'Select All' shown as Figure 3 What is the expected behavior? What went wrong? In above operations, the select menu is not displayed correctly and select unknown content. Did this work before? N/A Chrome version: 73.0.3645.0 Channel: stable OS Version: 6.0 Flash Version:
,
Dec 19
Unconfirmed -> Assigned as fanjinsong@ is working on it. Setting myself as the owner to ease tracking. This is caused by a small discrepancy between layout and editing that, if a block ends with a hard line break (<br> or preserved '\n'), it doesn't actually start a new line. However, editing allow highlighting hard line breaks, and somehow treats hard linebreaks as "there's something in the next line". I guess the fix should be: 1. Don't highlight a hard linebreak if it's the only rendered element in contenteditable 2. Don't allow select-all if a contenteditable has a hard line break as the only rendered child
,
Dec 19
Or maybe we just handle the BR-only case since this is quite an edge case...
,
Dec 20
Thanks for reply. I will continue to investigate the #1, this issue might require more than one patch ：）
,
Dec 20
I found that SelectionPopupControllerImpl::canPaste() also causes similar problems. https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java?rcl=af74e004f23bacfba169ac0c57f8a3ffb76cd930&l=784 Is it also necessary to check for the content likes '\n' for clipboard?
,
Dec 20
#5: I'm not very sure about this. "\n" is legit clipboard content, for example, one may copy a "\n" to clipboard. SelectionPopupControllerImpl doesn't look like the right level, either, since it doesn't work on DOM. Adding ctzsm@ since I'm not familiar with Android code.
,
Dec 20
,
Dec 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ee1dc03a5bb9d983f665d67a56ecfa7c7b9f309 commit 9ee1dc03a5bb9d983f665d67a56ecfa7c7b9f309 Author: Jinsong Fan <fanjinsong@sogou-inc.com> Date: Sat Dec 22 23:04:21 2018 Disable SelectAll for the editable contains a BR only When the editable contains a BR only, it appears as an empty line, in which case allowing select-all confuses users.The CL checks for this special-case. Bug: 916405 Change-Id: I394c83f10b7b3537973314d92123e629a9d7fd61 Reviewed-on: https://chromium-review.googlesource.com/c/1384324 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#618768} [modify] https://crrev.com/9ee1dc03a5bb9d983f665d67a56ecfa7c7b9f309/third_party/blink/renderer/core/editing/commands/editor_command.cc [add] https://crrev.com/9ee1dc03a5bb9d983f665d67a56ecfa7c7b9f309/third_party/blink/web_tests/editing/selection/select_all/select_all_contenteditable_br.html [add] https://crrev.com/9ee1dc03a5bb9d983f665d67a56ecfa7c7b9f309/third_party/blink/web_tests/editing/selection/select_all/select_all_contenteditable_br_only.html
,
Dec 28
Ｉ test more cases for <br> and the preserved '\n':
<html>
<body>
<div>Test</div>
<p>Bar</p>
<p>Foo</p>
<div>Test</div>
<div><pre>\n</pre></div>
<div>End</div>
<div contenteditable></div>
</body>
</html>
1. Focus the empty editable div, then click ‘break line’ on IME, the editable div has <div><br></div> <div><br></div> -> "Select All"
2. Copy the 'BR' between Bar and Foo, paste it to the editable div, then the editable div has <div><br></div> <div><br></div> -> "Paste | Select All"
3. Copy the preserved '\n', paste it to the editable div, then the editable div has <pre>\n</pre>, -> "Paste | Select All"
after clear "\n", the editable div has <pre><br>""</pre>, -> "Paste | Select All"
after clear "", the editable div ｂecomes empty again. -> "Paste"
Above tests, We might need a check for <div><br></div>
,
Jan 3
fanjinsong@: I'm confused by your comment #9 What exactly are the buggy cases there, and what should be their expected behaviors?
,
Jan 14
I'll close this issue next week if there's no more work to today.
,
Jan 15
Sorry for late reply. In the commnet #9, click "Enter" in the empty editable, it will produce <div><br></div> not <br> only, so "Select All" will be allowed. I mean need to do the same check for <div><br></div> like <br> only?
,
Jan 15
#12: Do you mean the following behavior 1. Start with <div contenteditable></div> 2. Press 'Enter' 3. Got <div contenteditable><div><br></div><div><br></div></div> 4. Select-all is enabled in the context menu This should be the intended behavior. Each <div><br></div> stands for an empty line so the HTML is correct. Users expect a '\n' in the contenteditable div, so select-all should be enabled.
,
Jan 16
Thanks for your explanation :) I have no other questions.
,
Jan 17
(5 days ago)
Thanks! Closing this issue as per #14.
,
Yesterday
(42 hours ago)
The NextAction date has arrived: 2019-01-21 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by fanjins...@sogou-inc.com
, Dec 19