63927 2011-07-05 02:25:16 -0700 [META] REGRESSION(r72052): Multiple placeholder regressions 2011-07-13 19:19:17 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) All All RESOLVED FIXED P2 Major --- 49452 51290 53740 54797 54814 63367 1 tkent webkit-unassigned adele darin dglazkov playmobil oldest_to_newest 432172 0 tkent 2011-07-05 02:25:16 -0700 [META] REGRESSION(r72052): Multiple placeholder regressions 432584 1 tkent 2011-07-05 20:19:02 -0700 A) I tried to represent a placeholder text as a shadow node, and replace the inner editable node with the placeholder node. The code change for this approach was really simple. However I had no good idea for implementing Bug 53740 behavior. - How to show the text caret with the placeholder text? - How to handle context menu on the placeholder node? B) I tried to implement another approach. Representing a placeholder text as a shadow node, it has display:none if the control has non-empty value, and show the placeholder node if the placeholder should be visible. This approach requires to add a lot of custom layout code in RenderTextControlSingleLine.cpp. I don't want to make it more complex ;-( C) Fixing text-align and line-height issues in the current implementation is not hard. I guess fixing the wrapping issue, Bug 51290, would be hard and boring. 432619 2 tkent 2011-07-05 22:12:12 -0700 D) Revting r72052 won't work well for Bug 53740. It would be hard to support editing context menu. 432793 3 dglazkov 2011-07-06 08:24:41 -0700 (In reply to comment #1) I think A is the right approach. > A) > I tried to represent a placeholder text as a shadow node, and replace the inner editable node with the placeholder node. The code change for this approach was really simple. > However I had no good idea for implementing Bug 53740 behavior. > - How to show the text caret with the placeholder text? > - How to handle context menu on the placeholder node? Can we position placeholder under the text input? Would this solve the issue? 433941 4 tkent 2011-07-08 00:57:06 -0700 (In reply to comment #3) > (In reply to comment #1) > > I think A is the right approach. > > > A) > > I tried to represent a placeholder text as a shadow node, and replace the inner editable node with the placeholder node. The code change for this approach was really simple. > > However I had no good idea for implementing Bug 53740 behavior. > > - How to show the text caret with the placeholder text? > > - How to handle context menu on the placeholder node? > > Can we position placeholder under the text input? Would this solve the issue? In A, the inner editable text node is detached while the placeholder is visible. So we can't do anything for editing. I'm trying B again. If we use position:absolute for the placeholder node, it's difficult to keep the correct position when the <input> is moved. If we use position:relative/static, <input> secures the intrinsic block for the placeholder node. I found a way to resolve this issue and minimize custom layout code. 434701 5 tkent 2011-07-11 00:28:10 -0700 Ok, I confirmed B resolved all of issues and did not break existing tests. I'll post a patch to Bug 64253. 434986 6 dglazkov 2011-07-11 10:33:07 -0700 (In reply to comment #5) > Ok, I confirmed B resolved all of issues and did not break existing tests. > I'll post a patch to Bug 64253. Great to hear. In retrospect, it does look like the sanest approach. 436870 7 tkent 2011-07-13 19:19:17 -0700 All dependent bugs have been fixed.