If an element has unicode-bidi: plaintext and no strong directional characters. By default it shoul…

If an element has unicode-bidi: plaintext and no strong directional characters. By default it should be LTR.

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

[eseidel, 2013-08-22 20:02:16]

lgtm

This looks great.

This is certainly putting isolate/plaintext code more in the critical path. 
Hopefully that will be OK.

[commit-bot: I haz the power, 2013-08-22 20:02:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/23112019/1

[eseidel, 2013-08-22 20:03:07]

https://codereview.chromium.org/23112019/diff/1/Source/core/html/HTMLElement.cpp
File Source/core/html/HTMLElement.cpp (left):

https://codereview.chromium.org/23112019/diff/1/Source/core/html/HTMLElement....
Source/core/html/HTMLElement.cpp:849: if (isHTMLTextFormControlElement(this)) {
There are other form controls other than <textarea>.  Are you sure this is OK to
remove w/ only your one modification to html.css?

[igoroliveira, 2013-08-22 21:23:28]

On 2013/08/22 20:03:07, eseidel wrote:
>
https://codereview.chromium.org/23112019/diff/1/Source/core/html/HTMLElement.cpp
> File Source/core/html/HTMLElement.cpp (left):
> 
>
https://codereview.chromium.org/23112019/diff/1/Source/core/html/HTMLElement....
> Source/core/html/HTMLElement.cpp:849: if (isHTMLTextFormControlElement(this))
{
> There are other form controls other than <textarea>.  Are you sure this is OK
to
> remove w/ only your one modification to html.css?

Yes. The code i removed was added by bug:
https://bugs.webkit.org/show_bug.cgi?id=65074
And it tried to fix the problem *just*  for textarea. Additionally the logic in
determineDirectionality just applies for textarea[1].

[1]http://www.w3.org/html/wg/drafts/html/master/dom.html#the-dir-attribute

[igoroliveira, 2013-08-22 21:33:17]

On 2013/08/22 21:23:28, Igor Oliveira wrote:
> On 2013/08/22 20:03:07, eseidel wrote:
> >
>
https://codereview.chromium.org/23112019/diff/1/Source/core/html/HTMLElement.cpp
> > File Source/core/html/HTMLElement.cpp (left):
> > 
> >
>
https://codereview.chromium.org/23112019/diff/1/Source/core/html/HTMLElement....
> > Source/core/html/HTMLElement.cpp:849: if
(isHTMLTextFormControlElement(this))
> {
> > There are other form controls other than <textarea>.  Are you sure this is
OK
> to
> > remove w/ only your one modification to html.css?
> 
> Yes. The code i removed was added by bug:
> https://bugs.webkit.org/show_bug.cgi?id=65074
> And it tried to fix the problem *just*  for textarea. Additionally the logic
in
> determineDirectionality just applies for textarea[1].
> 
> [1]http://www.w3.org/html/wg/drafts/html/master/dom.html#the-dir-attribute

I was wrong. Look likes my patch breaks input forms. Updating.

[igoroliveira, 2013-08-22 22:01:31]

Patch updated fixing input form regression.

[eseidel, 2013-08-22 23:32:12]

https://codereview.chromium.org/23112019/diff/10001/Source/core/html/HTMLElem...
File Source/core/html/HTMLElement.cpp (right):

https://codereview.chromium.org/23112019/diff/10001/Source/core/html/HTMLElem...
Source/core/html/HTMLElement.cpp:850: if (isHTMLTextFormControlElement(this) &&
!isHTMLTextAreaElement(this)) {
Can we get rid of this code for all other elements eventually?

Also, I would add a comment explaining why textarea is excluded here, noting
that it uses plaintext instead in html.css.

[igoroliveira, 2013-08-23 18:26:28]

On 2013/08/22 23:32:12, eseidel wrote:
>
https://codereview.chromium.org/23112019/diff/10001/Source/core/html/HTMLElem...
> File Source/core/html/HTMLElement.cpp (right):
> 
>
https://codereview.chromium.org/23112019/diff/10001/Source/core/html/HTMLElem...
> Source/core/html/HTMLElement.cpp:850: if (isHTMLTextFormControlElement(this)
&&
> !isHTMLTextAreaElement(this)) {
> Can we get rid of this code for all other elements eventually?

Yes, input forms should have the same css rule when type attribute is in the
Text, Search, Telephone, URL, or E-mail state[1].
However [2] does not define any rule. I will ping Tab or open a w3c bug about
it.

> 
> Also, I would add a comment explaining why textarea is excluded here, noting
> that it uses plaintext instead in html.css.

Since i changed the patch a bit i am not adding the a comment.


[1] http://www.w3.org/html/wg/drafts/html/master/dom.html#the-dir-attribute
[2] http://www.w3.org/TR/html51/rendering.html#bidirectional-text

[eseidel, 2013-08-23 18:57:48]

https://codereview.chromium.org/23112019/diff/15001/Source/core/html/HTMLElem...
File Source/core/html/HTMLElement.cpp (right):

https://codereview.chromium.org/23112019/diff/15001/Source/core/html/HTMLElem...
Source/core/html/HTMLElement.cpp:851: HTMLInputElement* inputElement =
toHTMLInputElement(const_cast<HTMLElement*>(this));
Seems we should just have a const-aware version of toHTMLInputElement, no?

[igoroliveira, 2013-08-23 21:16:27]

On 2013/08/23 18:57:48, eseidel - vacation until 9.3 wrote:
>
https://codereview.chromium.org/23112019/diff/15001/Source/core/html/HTMLElem...
> File Source/core/html/HTMLElement.cpp (right):
> 
>
https://codereview.chromium.org/23112019/diff/15001/Source/core/html/HTMLElem...
> Source/core/html/HTMLElement.cpp:851: HTMLInputElement* inputElement =
> toHTMLInputElement(const_cast<HTMLElement*>(this));
> Seems we should just have a const-aware version of toHTMLInputElement, no?

toHTMLInputElement has a const-aware version, however it is returning const
HTMLInputElement*:
inline const HTMLInputElement* toHTMLInputElement(const Node* node).

And i need a *non-const* return type and c++ does not support overload by return
type.

[eseidel, 2013-08-23 22:47:49]

lgtm

[commit-bot: I haz the power, 2013-08-23 22:48:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/23112019/15001

[commit-bot: I haz the power, 2013-08-24 00:49:25]

Change committed as 156664