Bug 69680 - Dir attribute is converted into direction property when merging paragraphs
Summary: Dir attribute is converted into direction property when merging paragraphs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 68168
  Show dependency treegraph
 
Reported: 2011-10-07 17:37 PDT by Ryosuke Niwa
Modified: 2011-10-11 18:06 PDT (History)
9 users (show)

See Also:


Attachments
Patch (13.41 KB, patch)
2011-10-11 15:42 PDT, Ryosuke Niwa
enrica: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-10-07 17:37:44 PDT
If a user presses delete key and merges two paragraphs between <div dir="ltr">X</div><div dir="rtl">Y</div> where X and Y can independently be LTR or RTL text, should we generate <div dir="ltr">XY</div> or <div dir="ltr">X<span dir="rtl">Y</div>?

WebKit currently adds direction property when copying a content inside an element with dir property. But this wouldn't work if the copied content was wrapped by span (need to add unicode-bidi: embed as well). I don't even know what the correct behavior is.

Test case (from the bug 45784): https://bug-45784-attachments.webkit.org/attachment.cgi?id=110125
Comment 1 Aharon (Vladimir) Lanin 2011-10-09 03:21:36 PDT
Caveat: below are my personal opinions. I do not think that there is a standard to follow here.

> should we generate <div dir="ltr">XY</div>
> or <div dir="ltr">X<span dir="rtl">Y</div>?

IMO, the former. The problem with the latter is that you are converting paragraph direction style to inline direction style. While paragraph direction is usually indicated by alignment, and most editors provide buttons to see it and control it, neither is the case for inline direction. Thus, later on, the user has no way of knowing that the <span dir=rtl> is there, and thus will be surprised when things work differently than they do normally. And even if the user figures out what's going on, he has no way of removing the inline direction style, and can not tell whether the caret is inside it or just next to it. This is all very different from the case with other styles, e.g. bolding, where the bolding is obvious in itself - and is made even more obvious by the status of its button, which also gives a way to control it.

(However, this issue should be revisited if WebKit were to start supporting a visualization of the bidi structure of the content during editing.)

> WebKit currently adds direction property when copying content
> inside an element with dir property. But this wouldn't work if
> the copied content was wrapped by span (need to add
> unicode-bidi: embed as well). I don't even know what the correct
> behavior is.

First, about the dir attribute vs direction & unicode-embded styles: stay true to what the source has. If it has mark-up like the dir attribute, use that mark-up. Only use CSS if it has something beyond the mark-up. 

Now regarding copying directional information on the copy command. Here too, I think the guiding principle should be to avoid creating inline direction. Thus, if you are going to be creating a <div> or <p>, I think it is very good to put a dir on it in accordance with the source's computed style (even if the source did not have it explicitly on that element). And if the source already has a <span dir=...>, then by all means copy it verbatim. But I do not think that it is good to create a span with direction when the source did not have it.
Comment 2 Ryosuke Niwa 2011-10-10 15:50:30 PDT
Thanks for the response, Aharon!

(In reply to comment #1)
> > should we generate <div dir="ltr">XY</div>
> > or <div dir="ltr">X<span dir="rtl">Y</div>?
> 
> IMO, the former. The problem with the latter is that you are converting paragraph direction style to inline direction style. While paragraph direction is usually indicated by alignment, and most editors provide buttons to see it and control it, neither is the case for inline direction.

Right. It's really hard for users to figure out that the latter is happening.

>Thus, later on, the user has no way of knowing that the <span dir=rtl> is there, and thus will be surprised when things work differently than they do normally. And even if the user figures out what's going on, he has no way of removing the inline direction style, and can not tell whether the caret is inside it or just next to it.

Safari provides a context menu to do this but I agree that it's not so user-friendly if the user had to use it very often after copy-paste.

> > WebKit currently adds direction property when copying content
> > inside an element with dir property. But this wouldn't work if
> > the copied content was wrapped by span (need to add
> > unicode-bidi: embed as well). I don't even know what the correct
> > behavior is.
> 
> First, about the dir attribute vs direction & unicode-embded styles: stay true to what the source has. If it has mark-up like the dir attribute, use that mark-up. Only use CSS if it has something beyond the mark-up. 

This issue is a little tangential in my opinion. It's same problem as WebKit converting b to <span style="font-weight: bold"> in some cases. We can probably tackle this in some other bug.

> Now regarding copying directional information on the copy command. Here too, I think the guiding principle should be to avoid creating inline direction. Thus, if you are going to be creating a <div> or <p>, I think it is very good to put a dir on it in accordance with the source's computed style (even if the source did not have it explicitly on that element). And if the source already has a <span dir=...>, then by all means copy it verbatim. But I do not think that it is good to create a span with direction when the source did not have it.

Here's a tricky case what if an user selected a part of block? e.g. ([ and ] indicate the start and the end of selection below)
<div dir="rtl">hello [world<p>WebKit</p>]</div>

In this case, p is a block element and it has RTL directionality inherited from the parent div. Which one of the following markups should we generate?
1. world<p>WebKit</p>
2. world<p dir="rtl">WebKit</p>
3. <div dir="rtl">world<p>WebKit</p></div>

I'd cross-out option 2 because "world" and "WebKit" would have different directionalities after paste and behave differently for <div dir="rtl">hello [world<br>WebKit]</div>
Comment 3 Aharon (Vladimir) Lanin 2011-10-11 07:10:05 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > > should we generate <div dir="ltr">XY</div>
> > > or <div dir="ltr">X<span dir="rtl">Y</div>?
> > 
> > IMO, the former. The problem with the latter is that you are converting
> > paragraph direction style to inline direction style. While paragraph
> > direction is usually indicated by alignment, and most editors provide
> > buttons to see it and control it, neither is the case for inline direction.
> > Thus, later on, the user has no way of knowing that the <span dir=rtl> is
> > there, and thus will be surprised when things work differently than they do
> > normally. And even if the user figures out what's going on, he has no way
> > of removing the inline direction style, and can not tell whether the caret
> > is inside it or just next to it.
> 
> Safari provides a context menu to do this

Wow, I did not realize this.

> but I agree that it's not so user-friendly if the user had to use it very
> often after copy-paste.

Indeed. To see that the inline direction style is there, one has to highlight *exactly* the text covered by the span (not one character more) and click for the context menu. Not very discoverable.

> > > WebKit currently adds direction property when copying content
> > > inside an element with dir property. But this wouldn't work if
> > > the copied content was wrapped by span (need to add
> > > unicode-bidi: embed as well). I don't even know what the correct
> > > behavior is.
> > 
> > First, about the dir attribute vs direction & unicode-embded styles: stay
> > true to what the source has. If it has mark-up like the dir attribute, use
> > that mark-up. Only use CSS if it has something beyond the mark-up. 
> 
> This issue is a little tangential in my opinion.

True.

> It's same problem as WebKit converting b to <span style="font-weight: bold">
> in some cases. We can probably tackle this in some other bug.

The problems are not of the same severity. Bolding is basically an issue of presentation, so style is the right way to do it (for which reason <b> had been deprecated at some point). Text direction is far more than presentation - it is the very nature of the text being displayed. The dir attribute is the right way to do it, and doing it in style alone is discouraged. But, as you said, this is not what this bug is about.

> > Now regarding copying directional information on the copy command. Here
> > too, I think the guiding principle should be to avoid creating inline
> > direction. Thus, if you are going to be creating a <div> or <p>, I think it
> > is very good to put a dir on it in accordance with the source's computed
> > style (even if the source did not have it explicitly on that element). And
> > if the source already has a <span dir=...>, then by all means copy it
> > verbatim. But I do not think that it is good to create a span with
> > direction when the source did not have it.
> 
> Here's a tricky case what if an user selected a part of block? e.g. ([ and ]
> indicate the start and the end of selection below)
> <div dir="rtl">hello [world<p>WebKit</p>]</div>
> 
> In this case, p is a block element and it has RTL directionality inherited
> from the parent div. Which one of the following markups should we generate?
> 1. world<p>WebKit</p>
> 2. world<p dir="rtl">WebKit</p>
> 3. <div dir="rtl">world<p>WebKit</p></div>
> 
> I'd cross-out option 2 because "world" and "WebKit" would have different
> directionalities after paste and behave differently for <div dir="rtl">hello
> [world<br>WebKit]</div>

Whatever you do, I do not think it should be a special case for directionality. That is, you should not come up with different element structures for two cases that only differ in elements having or not having a dir attribute. Probably the best way to think about it is to change the case to have a text-align style (which, by definition, is a block-level style) where it currently has a dir attribute and work backwards from there. So, what would you do for <div style="text-align:center">hello [world<p>WebKit</p>]</div>?

The good thing about asking that question is that you do not have to limit yourself to a supposed bidi expert like myself.
Comment 4 Aharon (Vladimir) Lanin 2011-10-11 07:18:43 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Safari provides a context menu to [set selection direction]
> 
> Wow, I did not realize this.
> 

BTW, this feature triggers a bug in Gmail. Let's say we have

<div dir=ltr>hello <span dir=rtl>RTL!</span> world</div>

When the user puts the caret inside the span, Gmail's LTR paragraph button becomes "non-pushed" and the RTL paragraph button becomes "pushed". This is already bad - the paragraph is *not* RTL. If one then clicks the LTR paragraph button, nothing happens - the "RTL!" is still displayed as "!RTL", while the LTR paragraph button stays non-pushed, and the RTL button stays pushed.
Comment 5 Ryosuke Niwa 2011-10-11 12:20:58 PDT
(In reply to comment #3)
> Whatever you do, I do not think it should be a special case for directionality. That is, you should not come up with different element structures for two cases that only differ in elements having or not having a dir attribute. Probably the best way to think about it is to change the case to have a text-align style (which, by definition, is a block-level style) where it currently has a dir attribute and work backwards from there. So, what would you do for <div style="text-align:center">hello [world<p>WebKit</p>]</div>?

That's a good approach. We don't preserve text-align in this case so we should probably do the same for directionality. Thanks for your help!
Comment 6 Ryosuke Niwa 2011-10-11 15:42:03 PDT
Created attachment 110590 [details]
Patch
Comment 7 Enrica Casucci 2011-10-11 17:01:37 PDT
Comment on attachment 110590 [details]
Patch

Looks good to me
Comment 8 Ryosuke Niwa 2011-10-11 17:05:27 PDT
Thanks for the review.
Comment 9 Ryosuke Niwa 2011-10-11 18:06:54 PDT
Committed r97205: <http://trac.webkit.org/changeset/97205>