Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2833313003:
Pass clickEvent to controlElement inside label if targetNode can't start selection (Closed)
DescriptionPass clickEvent to controlElement inside label if target node can't start selection.
On selection of any text in the HTML content, if any of the image
in a label is clicked, radio input is not selected.
This CL fixes this issue, the click events were not getting passed
to the control elements, as for few elements like image, update
selection is not done on single click as |CanMouseDownStartSelect()|
returns false. So this CL checks whether a selection could be done
on an element inside label.
BUG=637516
TEST=run_layout_test fast/forms/label/label-selection-by-textSelection-and-click.html
Review-Url: https://codereview.chromium.org/2833313003
Cr-Commit-Position: refs/heads/master@{#473151}
Committed: https://chromium.googlesource.com/chromium/src/+/5db23fe6d7151affeaf8b84ac2b16cf1603c456d
Patch Set 1 : Patch Set1 #Patch Set 2 : Added LayoutTestCase #
Total comments: 16
Patch Set 3 : Review Comments updated #
Total comments: 7
Patch Set 4 : patch set updated #
Total comments: 4
Patch Set 5 : updated Patch #
Total comments: 8
Patch Set 6 : Address Review Comments #
Messages
Total messages: 57 (36 generated)
Description was changed from ========== Label with img does not work correctly Label with img does not work correctly. On selection of any text in the HTML content, if any of the image in a Label is clicked, Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 ========== to ========== Label with img does not work correctly Label with img does not work correctly. On selection of any text in the HTML content, if any of the image in a Label is clicked, Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 R=SamsungPeerReview ==========
tanvir.rizvi@samsung.com changed reviewers: + shanmuga.m@samsung.com, srirama.m@samsung.com
The issue scenario is as below. There is a text Element, followed by a label element. Label element has in itself a image and a radio control element. If selection of text is done at any place, and then the image is selected, the radio button should get highlighted, but radio button do not get highlighted. Below is the patch description-: #1 SelectionController::HandleMousePressEventSingleClick() is responsible for updating the selection on single Click. This function has a check mouse_down_may_start_select_ based on which this function may/may-not move ahead on updating selection. If a click is on <img> element, this function do early return as check mouse_down_may_start_select_ is false. #2 In HTMLLabelElement, the concept that if a text on label element is selected by dragging over it , then control is not passed. #3 In the issue scenario, if text is selected, and click is done on <image> element, the due to above #1, the control is not getting passed down, while if the click is done on any text element, since selection is updated, the control is passed. #4. Since we are restricting update selection on image element. So while checking in Label Element we should again by pass the elements on which the selection can not be started. PTAL!!! I will update the layout text if you agree with this approach
The CQ bit was checked by tanvir.rizvi@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Label with img does not work correctly Label with img does not work correctly. On selection of any text in the HTML content, if any of the image in a Label is clicked, Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 R=SamsungPeerReview ========== to ========== On selection of any text in the HTML content, if any of the image in a Label is clicked,Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 R=SamsungPeerReview ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Peer review is fine, lets get the opinion from module owners.
Description was changed from ========== On selection of any text in the HTML content, if any of the image in a Label is clicked,Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 R=SamsungPeerReview ========== to ========== On selection of any text in the HTML content, if any of the image in a Label is clicked,Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 ==========
tanvir.rizvi@samsung.com changed reviewers: + yosin@chromium.org
Hi Yosin, PTAL! I have mentioned complete details as why i added this check in previous comments. Please let me know your opinion on this. I will add the Layout case accordingly. Thanks!!!
- Could you start description with summary? - Could you make summary to describe what this patch changes instead of current buggy fact, "Label with img does not work correctly" - Could you write bug number as "BUG=123456", "BUG" should be all upper case letter to make automatic link to crbug.com - Could you check bug number? It seems issue 637515 isn't related to this patch. http://crbug.com/637515 Chrome crashes on any page I try to open, incuding chrome://settings
Description was changed from ========== On selection of any text in the HTML content, if any of the image in a Label is clicked,Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. Bug=637515 ========== to ========== Pass ClickEvent to ControlElement inside LabelElement if TargetNode Can't Start Selection. On selection of any text in the HTML content, if any of the image in a Label is clicked,Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. BUG=637516 ==========
On 2017/04/25 07:14:20, yosin_UTC9 wrote: > - Could you start description with summary? > - Could you make summary to describe what this patch changes instead of current > buggy fact, "Label with img does not work correctly" > - Could you write bug number as "BUG=123456", "BUG" should be all upper case > letter to make > automatic link to http://crbug.com > - Could you check bug number? It seems issue 637515 isn't related to this patch. > > > http://crbug.com/637515 Chrome crashes on any page I try to open, incuding > chrome://settings Done.Sorry for the above Bug mismatch. PTAL! Thanks
Could you add test to verify this patch fix the issue and to avoid regression in the future? Thanks!
Added layoutTest case. PTAL! Thanks
https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html (right): https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:4: <p id="textId">Selected Text</p> you can remove id here and below and directly use querySelector to get the element. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:9: <script type="text/javascript"> remove type https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:11: var radioID = document.getElementById("radioId"); may be just use radio instead of radioID https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:12: var textID = document.getElementById("textId"); s/textID/text https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:23: testTouchMove.step(function () { nit:remove space before () https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:25: assert_equals(radioID.checked, true); use assert_true https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:33: [{source: "mouse", nit:space before "source" https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:45: var testTouchMove = async_test('Tests that Radio Input get checked if label clicked along with a text selection.'); s/get/gets https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:46: // This test run as follows. s/run/runs
PS updated for review comments. PTAL!!! https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html (right): https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:4: <p id="textId">Selected Text</p> On 2017/04/28 12:56:45, Srirama wrote: > you can remove id here and below and directly use querySelector to get the > element. Done. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:9: <script type="text/javascript"> On 2017/04/28 12:56:45, Srirama wrote: > remove type Done. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:23: testTouchMove.step(function () { On 2017/04/28 12:56:45, Srirama wrote: > nit:remove space before () Done. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:25: assert_equals(radioID.checked, true); On 2017/04/28 12:56:45, Srirama wrote: > use assert_true Done. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:33: [{source: "mouse", On 2017/04/28 12:56:45, Srirama wrote: > nit:space before "source" Done. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:45: var testTouchMove = async_test('Tests that Radio Input get checked if label clicked along with a text selection.'); On 2017/04/28 12:56:45, Srirama wrote: > s/get/gets Done. https://codereview.chromium.org/2833313003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:46: // This test run as follows. On 2017/04/28 12:56:45, Srirama wrote: > s/run/runs Done.
Peer review is fine
The CQ bit was checked by tanvir.rizvi@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html (right): https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:13: var radio = label.querySelector("input[name=a]"); nit: better to use 'input[type=radio]'. In this way we can omit "name" attribute. https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:37: { name: "pointerMove", x: textLeft+textWidth, y: textTop }, nit: s/+/ + / https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:46: var testTouchMove = async_test('Tests that Radio Input gets checked if label clicked along with a text selection.'); nit: Could you use double-quote here? Since, other parts of script use double-quote. https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right): https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:177: evt->target()->ToNode()->CanStartSelection()) Regarding a comment of this condition: "Check if there is a selection and click is not on the selection." Should we check whether Event#srcElement is LABEL element or not?
Should we check whether Event#srcElement is LABEL element or not? Yes i updated code to use isHTMLLabelElement now. This looks to be pretty straight :) instead of checking canStartSelection on a node PTAL!! Thanks. https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html (right): https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:13: var radio = label.querySelector("input[name=a]"); On 2017/05/08 03:25:56, yosin_UTC9 wrote: > nit: better to use 'input[type=radio]'. In this way we can omit "name" > attribute. Done. https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:37: { name: "pointerMove", x: textLeft+textWidth, y: textTop }, On 2017/05/08 03:25:56, yosin_UTC9 wrote: > nit: s/+/ + / Done. https://codereview.chromium.org/2833313003/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:46: var testTouchMove = async_test('Tests that Radio Input gets checked if label clicked along with a text selection.'); On 2017/05/08 03:25:56, yosin_UTC9 wrote: > nit: Could you use double-quote here? Since, other parts of script use > double-quote. Done.
yosin@chromium.org changed reviewers: + tkent@chromium.org
+tkent@ for Form control experts https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right): https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:167: isHTMLLabelElement(evt->target()->ToNode())) { I think we should check Event#srcElement in selection based on comment in the code: // Check if there is a selection and click is not on the // selection.
The CQ bit was checked by tkent@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right): https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:167: isHTMLLabelElement(evt->target()->ToNode())) { Does this work if <label> has a child element? e.g. <label><input type=checkbox> <b><img ...> Check this</b></label>
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right): https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:167: isHTMLLabelElement(evt->target()->ToNode())) { On 2017/05/09 02:14:26, tkent wrote: > Does this work if <label> has a child element? e.g. <label><input > type=checkbox> <b><img ...> Check this</b></label> 1) On using the label element as below <label><input type=checkbox> <b><img ...> Check this</b></label> "If there is selection of label element by dragging, no click event is passed. Also, no focus on control element" is failing. The reason is isHTMLLabelElement(evt->target()->ToNode()) fails in this case, as evt->target()->ToNode() is a <B> element. With PatchSet#3 i submitted this works fine. 2) There is one difference i observed with Firefox behaviour (Please check this for reference https://jsfiddle.net/tanvirR/egcpLffh/4/ ), When we select any text, and then click on <img> inside a Label, then control gets focused and the selection gets cleared. But in our case, the selection is not cleared. The reason as SelectionController::HandleSingleClick prevents it by the below condition if (!(inner_node && inner_node->GetLayoutObject() && mouse_down_may_start_select_)) return false; As on certain nodes canStartSelection returns false, like <img> Element. This is the reason i added evt->target()->ToNode()->CanStartSelection() in my PS#3 to by pass those nodes on which CanStartSelection() can not be done.
PTAL! Since IsHTMLLabelElement has some issues, i am using canStartSelection() logic here. Thanks. https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right): https://codereview.chromium.org/2833313003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:167: isHTMLLabelElement(evt->target()->ToNode())) { On 2017/05/09 00:56:06, yosin_UTC9 wrote: > I think we should check Event#srcElement in selection based on comment in the > code: > // Check if there is a selection and click is not on the > // selection. Hi Yosin, I couldn't get what you meant by " we should check Event#srcElement in selection ", Did you meant at SelectionController ?
CanStartSelection() looks reasonable to me. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html (right): https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:12: var label = document.querySelector("label"); nit: The following two |label.querySelector()| can be replaced with |document.querySelector()|, and we can remove the variable |label|. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:15: var targetTextRect = text.getBoundingClientRect(); nit: var targetTextRect = document.querySelector("p").getBoundingClientRect(); and remove the variable |text|. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:19: var targetImageRect = image.getBoundingClientRect(); nit: var targetImageRect = document.querySelector("img").getBoundingClientRect(); and remove the variable |image|. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:32: if (window.chrome && chrome.gpuBenchmarking) { This test should fail explicitly if there are no window.chrome and chrome.gpuBenchmarking. Also, using more aysnc_test features simplifies the code. testTouchMove.step(() => { assert_exists(window, "chrome"); assert_exists(chrome, "gpuBenchmarking"); ... chrome.gpuBenchmarking...(pointerActions, testTouchMove.step_func_done(() => { assert_equals(window.getSelection().toString(), "Selected Text"); assert_true(radio.checked); })); });
PTAL. Thanks https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html (right): https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:12: var label = document.querySelector("label"); On 2017/05/11 06:35:10, tkent wrote: > nit: > The following two |label.querySelector()| can be replaced with > |document.querySelector()|, and we can remove the variable |label|. Done. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:15: var targetTextRect = text.getBoundingClientRect(); On 2017/05/11 06:35:09, tkent wrote: > nit: > > var targetTextRect = document.querySelector("p").getBoundingClientRect(); > > and remove the variable |text|. Done. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:19: var targetImageRect = image.getBoundingClientRect(); On 2017/05/11 06:35:09, tkent wrote: > nit: > var targetImageRect = document.querySelector("img").getBoundingClientRect(); > > and remove the variable |image|. Done. https://codereview.chromium.org/2833313003/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/label/label-selection-by-textSelection-and-click.html:32: if (window.chrome && chrome.gpuBenchmarking) { On 2017/05/11 06:35:09, tkent wrote: > This test should fail explicitly if there are no window.chrome and > chrome.gpuBenchmarking. > Also, using more aysnc_test features simplifies the code. > > testTouchMove.step(() => { > assert_exists(window, "chrome"); > assert_exists(chrome, "gpuBenchmarking"); > ... > chrome.gpuBenchmarking...(pointerActions, testTouchMove.step_func_done(() => { > assert_equals(window.getSelection().toString(), "Selected Text"); > assert_true(radio.checked); > })); > }); Done.
The CQ bit was checked by tkent@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm. Please fix the CL description. The following words look like proper nouns, but they are not. Please do not use capital letters randomly. ClickEvent ControlElement LabelElement TargetNode Can't Start Selection. Label Radio Elements Label
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Pass ClickEvent to ControlElement inside LabelElement if TargetNode Can't Start Selection. On selection of any text in the HTML content, if any of the image in a Label is clicked,Radio input is not shown as selected. This CL fixes this issue, the click events were not getting passed to the control Elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside Label. BUG=637516 ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ==========
Description was changed from ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ==========
Description was changed from ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image update selection is not done on single click and CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ==========
Description was changed from ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ==========
Description was changed from ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 TEST=run_layout_test fast/forms/label/label-selection-by-textSelection-and-click.html ==========
Description was changed from ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as CanMouseDownStartSelect returns false. So this CL checks whether a selection could be done on a element inside label. BUG=637516 TEST=run_layout_test fast/forms/label/label-selection-by-textSelection-and-click.html ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as |CanMouseDownStartSelect()| returns false. So this CL checks whether a selection could be done on an element inside label. BUG=637516 TEST=run_layout_test fast/forms/label/label-selection-by-textSelection-and-click.html ==========
The CQ bit was checked by yosin@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1495177621947190,
"parent_rev": "f9dfdb6d9dcd8c5f987fd7114bff3ebee6b8785f", "commit_rev":
"5db23fe6d7151affeaf8b84ac2b16cf1603c456d"}
Message was sent while issue was closed.
Description was changed from ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as |CanMouseDownStartSelect()| returns false. So this CL checks whether a selection could be done on an element inside label. BUG=637516 TEST=run_layout_test fast/forms/label/label-selection-by-textSelection-and-click.html ========== to ========== Pass clickEvent to controlElement inside label if target node can't start selection. On selection of any text in the HTML content, if any of the image in a label is clicked, radio input is not selected. This CL fixes this issue, the click events were not getting passed to the control elements, as for few elements like image, update selection is not done on single click as |CanMouseDownStartSelect()| returns false. So this CL checks whether a selection could be done on an element inside label. BUG=637516 TEST=run_layout_test fast/forms/label/label-selection-by-textSelection-and-click.html Review-Url: https://codereview.chromium.org/2833313003 Cr-Commit-Position: refs/heads/master@{#473151} Committed: https://chromium.googlesource.com/chromium/src/+/5db23fe6d7151affeaf8b84ac2b1... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/5db23fe6d7151affeaf8b84ac2b1... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
