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
link

Issue 296863: 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.
 

Comment 1 by aharon.l...@gmail.com, Sep 24 2013

 Issue 108016  has been merged into this issue.

Comment 2 by eseidel@chromium.org, Sep 24 2013

Cc: igo...@sisa.samsung.com

Comment 3 by eseidel@chromium.org, Sep 24 2013

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.

Comment 5 by aharon.l...@gmail.com, Sep 25 2013

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.

Comment 6 by eseidel@chromium.org, Sep 25 2013

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.

Comment 8 by eseidel@chromium.org, Nov 13 2013

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.

Comment 9 by igo...@sisa.samsung.com, Nov 13 2013

No, it is OK.

Comment 10 by eseidel@chromium.org, Dec 18 2013

Comment 11 by behdad@chromium.org, Jun 18 2014

Cc: dominik....@intel.com h.jo...@samsung.com
Lets see if someone can pick this up.

Comment 12 by laforge@google.com, Jan 9 2015

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.

Comment 15 by jrobb...@google.com, Jun 15 2015

Cc: -dominik....@intel.com

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

Blocking: chromium:463348

Comment 17 by kojii@chromium.org, Nov 6 2015

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.

Comment 19 by jchaffraix@chromium.org, Dec 7 2015

Status: Available

Comment 20 by le...@chromium.org, Dec 8 2015

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?

Comment 23 by kojii@chromium.org, Aug 1 2017

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.

Comment 25 by kojii@chromium.org, Aug 2 2017

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.

Comment 26 by kojii@chromium.org, Sep 5 2017

Labels: -Type-Bug Type-Feature
Owner: ----
Status: Available (was: Assigned)

Comment 27 by robertma@chromium.org, Feb 26 2018

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.

Comment 29 by foolip@chromium.org, Feb 27 2018

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