[REGRESSION] Button cannot be clicked with text-indent : -99999999px after r143483.

[REGRESSION] Button cannot be clicked with text-indent : -99999999px after r143483.

Merged from http://trac.webkit.org/changeset/155546. 
This caused a bad repaint regression, when authors combine negative text-indent with overflow:hidden 
(which is what we recommend they do): bug 121137.
Revert the change, and add a new test to make sure it doesn't regress.

BUG=321892
TEST=
fast/repaint/negative-text-indent-with-overflow-hidden.html
fast/events/click-with-large-negative-text-indent.html
fast/repaint/paint-caret-in-div-with-negative-indent.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162590

[tkent, 2013-11-22 01:20:27]

Please add a test for click event issue which the bug mentions.

[seokju.kwon, 2013-11-22 02:10:22]

On 2013/11/22 01:20:27, tkent wrote:
> Please add a test for click event issue which the bug mentions.

Done.

[tkent, 2013-11-22 02:13:28]

On 2013/11/22 02:10:22, seokju.kwon wrote:
> On 2013/11/22 01:20:27, tkent wrote:
> > Please add a test for click event issue which the bug mentions.
> 
> Done.

> Test URL : http://jsfiddle.net/seokju/pkF2H/ 

No.  Please add a new layout test.

[seokju.kwon, 2013-11-22 04:07:58]

On 2013/11/22 02:13:28, tkent wrote:
> On 2013/11/22 02:10:22, seokju.kwon wrote:
> > On 2013/11/22 01:20:27, tkent wrote:
> > > Please add a test for click event issue which the bug mentions.
> > 
> > Done.
> 
> > Test URL : http://jsfiddle.net/seokju/pkF2H/ 
> 
> No.  Please add a new layout test.

Oops. I got it wrong. I've added new layout test.
Thanks!

[tkent, 2013-11-22 05:11:55]

https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
File LayoutTests/fast/events/click-with-large-negative-text-indent.html (right):

https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
LayoutTests/fast/events/click-with-large-negative-text-indent.html:7:
testRunner.dumpAsText();
We should call dumpAsText earlier.
e.g.

<script>
if (window.testRunner)
    testRunner.dumpAsText();
function test()
[
    ....

https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
LayoutTests/fast/events/click-with-large-negative-text-indent.html:8:
testRunner.waitUntilDone();
Calling waitUntilDone() and noifyDone() in one function makes no sense. Please
remove them.
You don't need to wait anything in this case because click event is synchronous
and the test will finish after the onload handler.

Also, you don't need to start test in onload handler. Just putting the test
script after the <button> should work.

https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
LayoutTests/fast/events/click-with-large-negative-text-indent.html:12:
eventSender.mouseMoveTo(testEle.offsetLeft+testEle.offsetWidth/2,
testEle.offsetTop+testEle.offsetHeight/2);
Need spaces around '+' and '/'

[seokju.kwon, 2013-11-22 05:35:50]

On 2013/11/22 05:11:55, tkent wrote:
>
https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
> File LayoutTests/fast/events/click-with-large-negative-text-indent.html
(right):
> 
>
https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
> LayoutTests/fast/events/click-with-large-negative-text-indent.html:7:
> testRunner.dumpAsText();
> We should call dumpAsText earlier.
> e.g.
> 
> <script>
> if (window.testRunner)
>     testRunner.dumpAsText();
> function test()
> [
>     ....
> 
>
https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
> LayoutTests/fast/events/click-with-large-negative-text-indent.html:8:
> testRunner.waitUntilDone();
> Calling waitUntilDone() and noifyDone() in one function makes no sense. Please
> remove them.
> You don't need to wait anything in this case because click event is
synchronous
> and the test will finish after the onload handler.
> 
> Also, you don't need to start test in onload handler. Just putting the test
> script after the <button> should work.
> 
>
https://codereview.chromium.org/81833005/diff/100001/LayoutTests/fast/events/...
> LayoutTests/fast/events/click-with-large-negative-text-indent.html:12:
> eventSender.mouseMoveTo(testEle.offsetLeft+testEle.offsetWidth/2,
> testEle.offsetTop+testEle.offsetHeight/2);
> Need spaces around '+' and '/'

Thanks,
Could you please clarify the meaning of "after the <button> should work" ?

[tkent, 2013-11-22 05:39:11]

On 2013/11/22 05:35:50, seokju.kwon wrote:
> Could you please clarify the meaning of "after the <button> should work" ?

It means the followings should work:

...
<body>
<button id="....>
<p id="output"></p>
<script>
function buttonClicked() { ... }
var testElement = ...
eventSender.mouseMoveTo(...)
eventSender.mouseDown();
eventSender.mouseUp();
</script>
</body>
</html>

[seokju.kwon, 2013-11-22 05:55:33]

On 2013/11/22 05:39:11, tkent wrote:
> On 2013/11/22 05:35:50, seokju.kwon wrote:
> > Could you please clarify the meaning of "after the <button> should work" ?
> 
> It means the followings should work:
> 
> ...
> <body>
> <button id="....>
> <p id="output"></p>
> <script>
> function buttonClicked() { ... }
> var testElement = ...
> eventSender.mouseMoveTo(...)
> eventSender.mouseDown();
> eventSender.mouseUp();
> </script>
> </body>
> </html>

Thanks, All done.

[tkent, 2013-11-22 05:59:01]

lgtm

[commit-bot: I haz the power, 2013-11-22 05:59:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/seokju.kwon@gmail.com/81833005/200001

[commit-bot: I haz the power, 2013-11-22 08:51:30]

Retried try job too often on mac_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_...

[commit-bot: I haz the power, 2013-11-22 09:30:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/seokju.kwon@gmail.com/81833005/330001

[commit-bot: I haz the power, 2013-11-22 11:41:48]

Retried try job too often on mac_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_...

[tkent, 2013-11-24 22:05:33]

Updating PNGs manually is troublesome.  Please put
paint-caret-in-div-with-negative-indent-expected.png to
LayoutTests/TestExpectations with [ NeedsRebaseline ].

[tkent, 2013-11-24 22:06:42]

On 2013/11/24 22:05:33, tkent wrote:
> Updating PNGs manually is troublesome.  Please put
> paint-caret-in-div-with-negative-indent-expected.png to

It should be fast/repaint/paint-caret-in-div-with-negative-indent.html, not
-expected.png.

[seokju.kwon, 2013-11-24 23:28:02]

On 2013/11/24 22:06:42, tkent wrote:
> On 2013/11/24 22:05:33, tkent wrote:
> > Updating PNGs manually is troublesome.  Please put
> > paint-caret-in-div-with-negative-indent-expected.png to
> 
> It should be fast/repaint/paint-caret-in-div-with-negative-indent.html, not
> -expected.png.

Thanks :-)

[commit-bot: I haz the power, 2013-11-24 23:28:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/seokju.kwon@gmail.com/81833005/490001

[commit-bot: I haz the power, 2013-11-24 23:28:23]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 582.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
f39e3adafe5a4e0964a5dddbf8562a3701a89b55..edf9f8cf191aa1661a3af5492d6aa0c368af24b1
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -582,3 +582,5 @@ crbug.com/245556
virtual/softwarecompositing/transitions/transform-on-large-laye
 crbug.com/321787 [ Win ]
virtual/gpu/fast/canvas/webgl/premultiplyalpha-test.html [ Pass Failure ]
 crbug.com/321793 [ Win ]
fast/frames/sandboxed-iframe-navigation-windowopen.html [ Pass Timeout ]
 crbug.com/321857 [ Win7 Debug ]
fast/filesystem/workers/file-writer-events-shared-worker.html [ Pass Failure ]
+
+Bug(seokju) fast/repaint/paint-caret-in-div-with-negative-indent.html [
NeedsRebaseline ]

[commit-bot: I haz the power, 2013-11-24 23:39:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/seokju.kwon@gmail.com/81833005/490002

[commit-bot: I haz the power, 2013-11-25 01:04:21]

Change committed as 162590