Bug 65617 - Make unicode-bidi:isolate the default for block elements instead of unicode-bidi:embed
Summary: Make unicode-bidi:isolate the default for block elements instead of unicode-b...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Noam Rosenthal
URL:
Keywords:
: 74988 (view as bug list)
Depends on: 50912
Blocks: 50910
  Show dependency treegraph
 
Reported: 2011-08-03 06:33 PDT by Aharon (Vladimir) Lanin
Modified: 2020-06-22 02:23 PDT (History)
18 users (show)

See Also:


Attachments
test case (591 bytes, text/html)
2011-08-03 06:57 PDT, Aharon (Vladimir) Lanin
no flags Details
test case (664 bytes, text/html)
2011-08-03 07:08 PDT, Aharon (Vladimir) Lanin
no flags Details
Patch (2.22 KB, patch)
2020-06-09 07:06 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (30.96 KB, patch)
2020-06-10 02:08 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aharon (Vladimir) Lanin 2011-08-03 06:33:08 PDT
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 Aharon (Vladimir) Lanin 2011-08-03 06:57:07 PDT
Created attachment 102778 [details]
test case
Comment 2 Aharon (Vladimir) Lanin 2011-08-03 07:08:04 PDT
Created attachment 102780 [details]
test case
Comment 3 Eric Seidel (no email) 2011-08-08 09:24:20 PDT
This is going to make the bidi-isolate code even more performance-critical than it already is. :(  Oh well.
Comment 4 Aharon (Vladimir) Lanin 2011-08-09 04:50:02 PDT
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 Eric Seidel (no email) 2011-09-29 11:45:42 PDT
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 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 7 Aharon (Vladimir) Lanin 2013-09-24 08:28:42 PDT
*** Bug 74988 has been marked as a duplicate of this bug. ***
Comment 8 Aharon (Vladimir) Lanin 2020-05-26 06:53:47 PDT
Please note that this change was made in Chrome and Mozilla years ago.
Comment 9 Noam Rosenthal 2020-05-26 06:54:51 PDT
(In reply to Aharon (Vladimir) Lanin from comment #8)
> Please note that this change was made in Chrome and Mozilla years ago.

It was in Mozilla... In Chrome I've just revived an old patch that was never merged.
https://chromium-review.googlesource.com/c/chromium/src/+/1081302
Comment 10 Aharon (Vladimir) Lanin 2020-05-26 08:10:28 PDT
Sorry, I got confused between https://bugs.chromium.org/p/chromium/issues/detail?id=296863 and https://bugs.chromium.org/p/chromium/issues/detail?id=391260. The latter, which is much more important than the former, was indeed fixed in 2016.

Note, however, that the corresponding WebKit bug, https://bugs.webkit.org/show_bug.cgi?id=134630, is still NEW. That's the more important one.
Comment 11 Noam Rosenthal 2020-05-26 08:11:36 PDT
(In reply to Aharon (Vladimir) Lanin from comment #10)
> Sorry, I got confused between
> https://bugs.chromium.org/p/chromium/issues/detail?id=296863 and
> https://bugs.chromium.org/p/chromium/issues/detail?id=391260. The latter,
> which is much more important than the former, was indeed fixed in 2016.
> 
> Note, however, that the corresponding WebKit bug,
> https://bugs.webkit.org/show_bug.cgi?id=134630, is still NEW. That's the
> more important one.

Thanks! Good to know.
Comment 12 Noam Rosenthal 2020-06-01 07:46:52 PDT
New discussion in WhatWG: https://github.com/whatwg/html/issues/5594
Comment 13 Noam Rosenthal 2020-06-02 00:42:11 PDT
(In reply to Noam Rosenthal from comment #11)
> (In reply to Aharon (Vladimir) Lanin from comment #10)
> > Sorry, I got confused between
> > https://bugs.chromium.org/p/chromium/issues/detail?id=296863 and
> > https://bugs.chromium.org/p/chromium/issues/detail?id=391260. The latter,
> > which is much more important than the former, was indeed fixed in 2016.
> > 
> > Note, however, that the corresponding WebKit bug,
> > https://bugs.webkit.org/show_bug.cgi?id=134630, is still NEW. That's the
> > more important one.
> 
https://bugs.webkit.org/show_bug.cgi?id=134630 has been resolved. Waiting for consensus on https://github.com/whatwg/html/issues/5594 before tackling this one.
Comment 14 Noam Rosenthal 2020-06-09 07:06:01 PDT
Created attachment 401438 [details]
Patch
Comment 15 Noam Rosenthal 2020-06-09 07:06:57 PDT
(In reply to Noam Rosenthal from comment #14)
> Created attachment 401438 [details]
> Patch

Uploaded a patch so that EWS can find all kinds of platform-specific failing tests.
Comment 16 Noam Rosenthal 2020-06-10 02:08:12 PDT
Created attachment 401523 [details]
Patch
Comment 17 EWS Watchlist 2020-06-10 02:08:56 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 18 Darin Adler 2020-06-10 11:11:00 PDT
Comment on attachment 401523 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401523&action=review

> Source/WebCore/css/html.css:1279
> -bdi, output {
> +address, blockquote, center, div, figure, figcaption, footer, form, header, hr,
> +legend, listing, main, p, plaintext, pre, summary, xmp, article, aside, h1, h2,
> +h3, h4, h5, h6, hgroup, nav, section, table, caption, colgroup, col, thead,
> +tbody, tfoot, tr, td, th, dir, dd, dl, dt, menu, ol, ul, li, bdi, output {
>      unicode-bidi: isolate;
>  }

Why not put "unicode-bidi: isolate" right after each "display: block", since the rule is that they go together? If we did that we could see that it’s done correctly. When done this way, to verify it’s correct we have to carefully scan this list to notice that nothing extra is included and nothing is left out, which seems impractical.
Comment 19 Noam Rosenthal 2020-06-11 01:37:28 PDT
(In reply to Darin Adler from comment #18)
> Comment on attachment 401523 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401523&action=review
> 
> > Source/WebCore/css/html.css:1279
> > -bdi, output {
> > +address, blockquote, center, div, figure, figcaption, footer, form, header, hr,
> > +legend, listing, main, p, plaintext, pre, summary, xmp, article, aside, h1, h2,
> > +h3, h4, h5, h6, hgroup, nav, section, table, caption, colgroup, col, thead,
> > +tbody, tfoot, tr, td, th, dir, dd, dl, dt, menu, ol, ul, li, bdi, output {
> >      unicode-bidi: isolate;
> >  }
> 
> Why not put "unicode-bidi: isolate" right after each "display: block", since
> the rule is that they go together? If we did that we could see that it’s
> done correctly. When done this way, to verify it’s correct we have to
> carefully scan this list to notice that nothing extra is included and
> nothing is left out, which seems impractical.

It's copied directly from https://html.spec.whatwg.org/multipage/rendering.html#bidi-rendering, I thought it would be easier to keep them in sync-ish and track later if we keep them similar.