1366620 - Reversed "Day, Month, Year" Hebrew text in <input type=date>

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Itiel]

Attachment: Reverted text

See attached screenshot, taken from latest Nightly.

STR:
1. Install latest Nightly, Hebrew locale
2. Open a new tab, enter data:text/html,<input type=date>

The Hebrew text there is inverted, so instead of "הנש.שדוח.םוי", there should be "שנה.חודש.יום".

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Jessica, seems like the component order is correct, but tokens have the wrong directionality?

Comment 2

[Jessica Jong [:jessica] (inactive)]

Hi ItielMaN, can I know the your locale code, since using `he` as locale, the date is displayed with arabic numbers.
`console.log(new Intl.DateTimeFormat('he').format(Date.now()));` gives `24.5.2017`.
And, have you tried it on Chrome?
Thanks.

Comment 3

[Itiel]

(In reply to Jessica Jong [:jessica] from comment #2)
> Hi ItielMaN, can I know the your locale code, since using `he` as locale,
> the date is displayed with arabic numbers.
> `console.log(new Intl.DateTimeFormat('he').format(Date.now()));` gives
> `24.5.2017`.
> And, have you tried it on Chrome?
> Thanks.

The dates actually look good, I was referring to the placeholders- the actual text "Day.Month,Year" (in Hebrew it should be "שנה.חודש.יום") before the user select a date.
Currently it's like you'd see the text in English as "yaD.htnoM.raeY".

Comment 4

[Jessica Jong [:jessica] (inactive)]

Thanks ItielMaN for the clarification. I think this is because we use `unicode-bidi: override` with direction `ltr`. Maybe we should use `unicode-bidi: embed` instead.

Comment 5

[Jessica Jong [:jessica] (inactive)]

Attachment: Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.

Review commit: https://reviewboard.mozilla.org/r/143058/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/143058/

Comment 6

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8871578 [details]
Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143058/diff/1-2/

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Why is the `field` direction: ltr instead of being picked based on the script direction?

Comment 8

[Jessica Jong [:jessica] (inactive)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #7)
> Why is the `field` direction: ltr instead of being picked based on the
> script direction?

Numbers are LTR, we just force it to be LTR so that the order of the fields doesn't change when the field is empty (with placeholder), otherwise, you'll notice that the order of the fields keeps changing while you enter values in the fields.
Opening an additional level of embedding (by using `embed`), will still make the placeholder text displayed based on its direction.

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8871578 [details]
Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.

rubberstamping r+ then. The code looks good and I trust :jessica's logic. :)

Comment 10

[Jessica Jong [:jessica] (inactive)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #9)
> Comment on attachment 8871578 [details]
> Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.
> 
> rubberstamping r+ then. The code looks good and I trust :jessica's logic. :)

Thanks zibi. I have to say I'm still very unfamiliar with all this bidi thing, but this seems to give the right result. :)
Date and time inputs are going to be enabled by default on Nightly this week, so hopefully, we'll have more testers to find out if there is any other issue.

Comment 11

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8871578 [details]
Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143058/diff/2-3/

Comment 12

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8871578 [details]
Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.

Carrying r+, since I only updated the comment.

Comment 13

[Jessica Jong [:jessica] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec2fe54198558b7b21822d6430a6c069f667c811&group_state=expanded

Comment 14

[Carsten Book [:Tomcat]]

need r+ also on mozreview so that we can use autoland

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8871578 [details]
Bug 1366620 - Use `unicode-bidi: embed` to fix reversed hebrew placeholders.

https://reviewboard.mozilla.org/r/143058/#review149132

Comment 16

[Jessica Jong [:jessica] (inactive)]

Thanks :zibi. Forwarding ni to :Tomcat.

Comment 17

[Carsten Book [:Tomcat]]

thanks should be pushed now :)

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50545ad26ef4
Use `unicode-bidi: embed` to fix reversed hebrew placeholders. r=gandalf

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/50545ad26ef4

Comment 20

[roxleitan]

Verified fixed on the latest Nightly 55.0a1 (2017-06-07) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.