third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall.html - Issue 1499063002: input's placeholder is inconsistently laid out

Side by Side Diff: third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall.html

Issue 1499063002: input's placeholder is inconsistently laid out (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « third_party/WebKit/LayoutTests/TestExpectations ('k') | third_party/WebKit/LayoutTests/fast/input/placeholder-wrongly-placed-if-too-tall-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <style>

	3 input {

	4 height: 26px;

	5 line-height: 29px;

	6 padding: 0 5px;

	7 border: none;

	8 border-bottom: 1px solid #888;

	9 outline: none;

	10 }

	11 </style>

	12 <div>Both inputs should be of the same size and the placeholder should be at the same place.</div>

	13 <div id="console"></div>

	14 <input type="text" placeholder="placholder" data-offset-y="8">
	mstensho (USE GERRIT) 2015/12/04 12:55:54 Who is Plach, and why is he older? Who is Plach, and why is he older? Julien - ping for review 2015/12/04 14:45:42 http://theinfosphere.org/Claw-Plach !!! Show quoted text On 2015/12/04 at 12:55:54, mstensho wrote: > Who is Plach, and why is he older? http://theinfosphere.org/Claw-Plach !!!
	15 <input type="text" placeholder="placholder" data-offset-y="8">

	16 <script src="../../resources/check-layout.js"></script>
	mstensho (USE GERRIT) 2015/12/04 12:55:54 There are attributes in this document that check-l There are attributes in this document that check-layout.js would recognize, but checkLayout() isn't called, so it's just a waste. I suspect that you changed your mind at some point, and ended up with checkInputs() instead. Julien - ping for review 2015/12/04 14:45:42 Doh', you're right. I tried to make it a checkLayo Show quoted text On 2015/12/04 at 12:55:54, mstensho wrote: > There are attributes in this document that check-layout.js would recognize, but checkLayout() isn't called, so it's just a waste. > > I suspect that you changed your mind at some point, and ended up with checkInputs() instead. Doh', you're right. I tried to make it a checkLayout test but checkLayout removes the bug in a manner I couldn't understand through code inspection.
	17 <script>

	18 if (window.testRunner) {

	19 testRunner.dumpAsText();

	20 testRunner.waitUntilDone();

	21 }

	22

	23 var console = document.getElementById("console");

	24 var inputs = document.getElementsByTagName("input");

	25

	26 function log(msg)

	27 {

	28 console.appendChild(document.createTextNode(msg));

	29 console.appendChild(document.createElement("br"));

	30 }

	31

	32 function checkInputs()

	33 {

	34 var rect0 = inputs[0].getBoundingClientRect();

	35 var rect1 = inputs[1].getBoundingClientRect();

	36 if (rect0.top === rect1.top) {

	37 log("PASSED, same top");
	mstensho (USE GERRIT) 2015/12/04 12:55:54 Couldn't you just use the shouldBe() thing in js-t Couldn't you just use the shouldBe() thing in js-test.js instead? Julien - ping for review 2015/12/04 14:45:42 Done! Show quoted text On 2015/12/04 at 12:55:54, mstensho wrote: > Couldn't you just use the shouldBe() thing in js-test.js instead? Done!
	38 } else {

	39 log("FAILED top: got " + rect0.top + " and " + rect1.top);

	40 }

	41

	42 if (rect0.height === rect1.height) {

	43 log("PASSED, same height");

	44 } else {

	45 log("FAILED height: got " + rect0.height + " and " + rect1.height);

	46 }

	47

	48 if (window.testRunner)

	49 testRunner.notifyDone();

	50 }

	51

	52 inputs[1].focus();

	53 // Forcing a layout in this frame makes the issue disappear.
	mstensho (USE GERRIT) 2015/12/04 12:55:54 Scary. But I guess there's no need to explain this Scary. But I guess there's no need to explain this weirdness, since you just fixed it, right? Julien - ping for review 2015/12/04 14:45:42 It's more of a warning to reviewer. I still think Show quoted text On 2015/12/04 at 12:55:54, mstensho wrote: > Scary. But I guess there's no need to explain this weirdness, since you just fixed it, right? It's more of a warning to reviewer. I still think it's important to mention this weirdness. mstensho (USE GERRIT) 2015/12/04 14:57:35 Absolutely. Sorry about being unclear. What I didn Show quoted text On 2015/12/04 14:45:42, Julien Chaffraix - PST wrote: > On 2015/12/04 at 12:55:54, mstensho wrote: > > Scary. But I guess there's no need to explain this weirdness, since you just > fixed it, right? > > It's more of a warning to reviewer. I still think it's important to mention this > weirdness. Absolutely. Sorry about being unclear. What I didn't write in my previous comment was that my initial thoughts were "WTF! Julien, please write an entire book about this, please! I want to know everything about this!", but then I realized that you were probably fixing this very piece of weirdness. And then I thought that one sentence (which is what you have provided) is just enough, and that a book on the topic would be TMI. :)
	54 window.requestAnimationFrame(checkInputs);

	55 </script>

OLD	NEW