Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(583)

Issue 16796004: REGRESSION(r147612): Double/triple click selection on input[type=search] doesn't paint selection co… (Closed)

Created:
7 years, 6 months ago by tkent
Modified:
7 years, 6 months ago
Reviewers:
Hajime Morrita, ojan
CC:
blink-reviews, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering
Visibility:
Public.

Description

REGRESSION(r147612): Double/triple click selection on input[type=search] doesn't paint selection correctly. r147612 added setNeedsLayout(true, MarkOnlyThis) for TextControlInnerTextElement renderer. It's ok for input[type=text] because RenderTextControlSingleLine also has needsLayout flag, and it has only the inner text renderer as a child. As for input[type=search], it doesn't work. Renderer structure for input[type=search] is: RenderTextControlSingleLine └Renderer for TextControlInnerContainer └Renderer for TextControlInnerElement └Renderer for TextControlInnerTextElement After r147612, there is a case that only RenderTextControlSingleLine and TextControlInnerTextElement renderer have needsLayout flag, and others don't. It won't trigger layout for TextControlInnerTextElement renderer. We should set needsLayout flags of TextControlInnerContainer renderer and TextControlInnerElement renderer by MarkContainingBlockChain. BUG=247600 TEST=manual: See crbug.com/247600 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152272

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M Source/core/rendering/RenderTextControlSingleLine.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
tkent
This is a regression by http://src.chromium.org/viewvc/blink?revision=147612&view=revision . I couldn't make a layout test for this. ...
7 years, 6 months ago (2013-06-12 02:17:14 UTC) #1
Hajime Morrita
Ok, lgtm.
7 years, 6 months ago (2013-06-12 07:00:28 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tkent@chromium.org/16796004/1
7 years, 6 months ago (2013-06-12 07:43:33 UTC) #3
commit-bot: I haz the power
7 years, 6 months ago (2013-06-12 07:47:16 UTC) #4
Message was sent while issue was closed.
Change committed as 152272

Powered by Google App Engine
This is Rietveld 408576698