New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 6 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Compat
RTL

Blocking:
issue 463348



Sign in to add a comment

Make unicode-bidi:isolate the default for block elements instead of unicode-bidi:embed

Reported by aharon.l...@gmail.com, Sep 23 2013

Issue description

Migrated from WebKit Bugzilla: https://bugs.webkit.org/show_bug.cgi?id=65617

Description From Aharon (Vladimir) Lanin 2011-08-03 06:33:08 PST
When a block element is set to display:inline, it is desirable for it to behave bidi-wise as it would if it were display:inline-block. Under CSS2.1, there was no way to accomplish this, and unicode-bidi:embed was used for lack of a better choice. Now that we have unicode-bidi:isolate, it should be the default instead, as the HTML5 spec (http://dev.w3.org/html5/spec/Overview.html#non-replaced-elements) says it should be.

Comment #1 From Aharon (Vladimir) Lanin 2011-08-03 06:57:07 PST
Created attachment https://bugs.webkit.org/attachment.cgi?id=102778
test case

Comment #2 From Aharon (Vladimir) Lanin 2011-08-03 07:08:04 PST
Created attachment https://bugs.webkit.org/attachment.cgi?id=102780
test case

Comment #3 From Eric Seidel 2011-08-08 09:24:20 PST
This is going to make the bidi-isolate code even more performance-critical than it already is. :(  Oh well.

Comment #4 From Aharon (Vladimir) Lanin 2011-08-09 04:50:02 PST
Please note that unicode-bidi:isolate should not have any effect (and thus can be completely ignored) on elements that already constitute separate bidi paragraphs, e.g. elements with display other than inline, or that have float, or that have absolute position, etc. The default is meant for the case when a "block" element like div gets assigned display:inline. This should be a fairly rare case.

Comment #5 From Eric Seidel 2011-09-29 11:45:42 PST
I suspect this may be as simple as adding the CSS from the HTML5 spec into html.css in WebCore:
http://www.whatwg.org/specs/web-apps/current-work/multipage/rendering.html#bidirectional-text
http://trac.webkit.org/browser/trunk/Source/WebCore/css/html.css

The important piece is of course the LayoutTests which get landed with any change.

Comment #6 From Aharon (Vladimir) Lanin 2011-11-17 02:13:55 PST
The change needs to be effective for "block" elements whether or not they have a dir attribute. Rules like
  div {display:block; unicode-bidi:isolate;}
are not sufficient because the generic [dir] {unicode-bidi:embed;} is considered more specific and overrides them for elements with a dir attribute. See http://www.w3.org/Bugs/Public/show_bug.cgi?id=14850.
 
 Issue 108016  has been merged into this issue.
Cc: igo...@sisa.samsung.com
Cc: le...@chromium.org

Comment 4 by le...@chromium.org, Sep 24 2013

It'll be super important to have performance test coverage. As Eric said, where we stand now, this will be a performance regression, but I'm not sure how much, and don't currently have a metric to burn down. I'm happy to code up something to look into the implications, since we can test this now. 
If it really does cause a performance regression, there is something wrong in the implementation, since unicode-bidi:isolate (just like unicode-bidi:embed) is not supposed to have any effect on an element that is not inline.
I'm sure there are many things wrong in the implementation. :)  That said, I think we shouldn't be scared, and we should just throw this CSS in html.css and see what (if anything) breaks.  BlinkON is distracting folks today.

Comment 7 by le...@chromium.org, Sep 26 2013

Aharon is right, of course. I was definitely distracted.

We should just turn this on.
Cc: sanjoy....@samsung.com
Igor: any reason you can think of we shouldn't just post a patch to html.css to turn this on?  I guess I'll do that tomorrow morning since it should be trivial.
No, it is OK. 
Here is the review link:
https://codereview.chromium.org/61733019/
Cc: dominik....@intel.com h.jo...@samsung.com
Lets see if someone can pick this up.
Labels: -Cr-Blink-Rendering Cr-Blink-Layout
Migrate from Cr-Blink-Rendering to Cr-Blink-Layout

Comment 13 by laforge@google.com, Jun 13 2015

Cc: drott@chromium.org

Comment 14 by laforge@google.com, Jun 13 2015

Attempting to remove dominik.rottsches@intel.com from cc.
Cc: -dominik....@intel.com

Comment 16 by ebra...@gnu.org, Nov 6 2015

Blocking: chromium:463348
Cc: kojii@chromium.org

Comment 18 by ebra...@gnu.org, Nov 6 2015

There is another WIP for this on https://codereview.chromium.org/1237143003/ and from tests IMO it looks promising.
Status: Available
Owner: kojii@chromium.org
Status: Assigned

Comment 21 by tkent@chromium.org, May 23 2016

Components: -Blink>RTL
Labels: RTL
Removing Blink>RTL.

Comment 22 by e...@chromium.org, Aug 1 2017

Is this still relevant?
Labels: -Pri-2 Pri-3
Yeah, this is about when author sets 'display: inline' to block elements such as <p>, they automatically create isolation. We implemented the "dir" attribute to isolate, but not this one.

Comment 24 by ebra...@gnu.org, Aug 2 2017

I believe https://codereview.chromium.org/1237143003/ still can be revived with not so huge effort.
Oh, I wasn't aware there's a WIP, thank you as always to make our bidi support better.

If you can revive it, that's great and I'm happy to review. Otherwise I might try to find some time, but my backlog is rather long currently.
Labels: -Type-Bug Type-Feature
Owner: ----
Status: Available (was: Assigned)
Cc: smcgruer@chromium.org robertma@chromium.org foolip@chromium.org
 Issue 816626  has been merged into this issue.

Comment 28 by ebra...@gnu.org, Feb 27 2018

I still believe https://codereview.chromium.org/1237143003/ can be revived if it could find an owner.
Do you mean a reviewer, or someone to upload the patch?

Comment 30 by ebra...@gnu.org, Feb 28 2018

Labels: -Type-Feature Type-Compat
Someone to re-upload the patch and fixes minor issue (my experience on last time) it may causes on tests.

Sign in to add a comment