BiDi: Setting document.dir has no effect on <html dir="rtl">

RESOLVED FIXED in mozilla23

Status

()

Core
Layout: Text
RESOLVED FIXED
15 years ago
2 years ago

People

(Reporter: Kyae-Young Kim, Assigned: smontagu)

Tracking

({dev-doc-needed, rtl})

Trunk
mozilla23
dev-doc-needed, rtl
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [oracle-nls])

Attachments

(4 attachments)

(Reporter)

Description

15 years ago
Env. : Mozilla 1.0, Windows NT/2000,

Des. : The LTR direction of windows does not support

Reproducible step:   
    1) Load test case in IE 5.5
    2) Click the Window Direction button
    3) Window will be fliped for LTR direction
    4) It does not support in Mozilla 1.0
(Reporter)

Comment 1

15 years ago
Created attachment 87467 [details]
Test case of 151407

Comment 2

15 years ago
CONFIRMED with moz 1.0 on win98
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

15 years ago
Adding regression keyword. I remember seeing something just like this working in
the past.
Keywords: regression
(Assignee)

Comment 4

15 years ago
The page that I was thinking of was http://www.qsm.co.il/Hebrew/HebrewTest/rtl.htm,
which still works. The significant difference seems to be that that page uses
document.dir while the attachment here uses top.document.dir

Updated

13 years ago
Whiteboard: [oracle-nls]
Changing summary to be more informative; removing "regression" keyword per
comment #4.
Keywords: regression
Summary: Bidi : The LTR direction of windows does not support → BiDi: Setting top.document.dir has no effect
(Assignee)

Comment 6

12 years ago
(In reply to comment #4)
> The page that I was thinking of was
http://www.qsm.co.il/Hebrew/HebrewTest/rtl.htm,
> which still works. The significant difference seems to be that that page uses
> document.dir while the attachment here uses top.document.dir

That was wrong. The significant difference is that the attachment here has 
<HTML dir="rtl"> and the page at www.qsm.co.il just has <HTML>.
(Assignee)

Updated

12 years ago
Depends on: 300270
(Assignee)

Updated

12 years ago
Summary: BiDi: Setting top.document.dir has no effect → BiDi: Setting document.dir has no effect on <html dir="rtl">
(Assignee)

Comment 7

12 years ago
The same is true in reverse: <html dir="rtl"> does not set document.dir to rtl
*** Bug 256470 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

12 years ago
Blocks: 312023

Updated

11 years ago
OS: Windows NT → All
Hardware: PC → All
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl

Updated

9 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text

Updated

6 years ago
Assignee: mozilla → nobody

Comment 10

5 years ago
This bug is now over a decade old.  Confirming error on OS X in Firefox.

The behaviour has become so well-known that it has now reached the documentation:
https://developer.mozilla.org/en-US/docs/DOM/Document.dir
http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#dom-document-dir says:

The dir IDL attribute on Document objects must reflect the dir content attribute of the html element, if any, limited to only known values. If there is no such element, then the attribute must return the empty string and do nothing on setting.

Comment 12

5 years ago
A workaround is to get or set:
  document.body.parentNode.dir
instead of:
  document.dir
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #11)
> http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.
> html#dom-document-dir says:
> 
> The dir IDL attribute on Document objects must reflect the dir content
> attribute of the html element, if any, limited to only known values. If
> there is no such element, then the attribute must return the empty string
> and do nothing on setting.

Hmm, given that, this is a valid bug, right?  The problem is that nsIDocument::SetDir just doesn't set the attribute on the root element, it just uses SetDirectionality on it.

Updated

4 years ago
Component: Layout: Text → DOM: Core & HTML
(Assignee)

Comment 14

4 years ago
Created attachment 740091 [details] [diff] [review]
Patch

I believe this makes our behaviour compatible with the HTML5 spec at http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#dom-dir

The patch removes the directionality property on nsIDocument, which is now redundant, and makes the dir IDL attribute on documents reflect the dir attribute of the html element, rather than the root element (this makes an existing test fail, see following patch).

It also stops using the bidi.direction config option to determine the default direction, in response to various comments in https://www.w3.org/Bugs/Public/show_bug.cgi?id=18340. I'll remove other uses of option in a follow-up bug.

Note that document.dir can now return four possible values: "auto", "ltr", "rtl", or the empty string. Previously it would always return either "ltr" or "rtl".
Assignee: nobody → smontagu
Attachment #740091 - Flags: review?(ehsan)
(Assignee)

Comment 15

4 years ago
Created attachment 740092 [details] [diff] [review]
Changes to existing tests
Attachment #740092 - Flags: review?(Ms2ger)
Comment on attachment 740091 [details] [diff] [review]
Patch

Review of attachment 740091 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!
Attachment #740091 - Flags: review?(ehsan) → review+
Comment on attachment 740092 [details] [diff] [review]
Changes to existing tests

Review of attachment 740092 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #740092 - Flags: review?(Ms2ger) → review+
Comment on attachment 740091 [details] [diff] [review]
Patch

Review of attachment 740091 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsDocument.cpp
@@ +6253,5 @@
>    return NS_OK;
>  }
>  
>  void
> +nsIDocument::GetDir(nsAString& aDirection)

I'd prefer making GetHtmlElement() const.

@@ +6283,5 @@
>    return rv.ErrorCode();
>  }
>  
>  void
>  nsIDocument::SetDir(const nsAString& aDirection, ErrorResult& rv)

Drop the ErrorResult here and the [SetterThrows] from Document.webidl
(Assignee)

Comment 19

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3165dffa51c
https://hg.mozilla.org/integration/mozilla-inbound/rev/b17e0eb827c0
Component: DOM: Core & HTML → Layout: Text
Flags: in-testsuite+
Was there a reason to not just call GetDir on the root element instead of doing a GetAttr and then comparing to a copy of the dir enum table?
(Assignee)

Comment 21

4 years ago
only that Element doesn't have GetDir
(Assignee)

Comment 22

4 years ago
Created attachment 740656 [details] [diff] [review]
GetDir on root elelement

would this have been better?
Attachment #740656 - Flags: review?(bzbarsky)
(In reply to comment #22)
> Created attachment 740656 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=740656&action=edit
> GetDir on root elelement
> 
> would this have been better?

Do we know that the root element is always going to be an HTML element?
(Assignee)

Comment 24

4 years ago
We know that it's an <html> element. Is that equivalent?
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #23)
> (In reply to comment #22)
> > Created attachment 740656 [details] [diff] [review]
> >   --> https://bugzilla.mozilla.org/attachment.cgi?id=740656&action=edit
> > GetDir on root elelement
> > 
> > would this have been better?
> 
> Do we know that the root element is always going to be an HTML element?

We know that it's an html element in the HTML namespace, so, yes. I guess we could make GetHtmlElement() return something more specific, but I don't know if all callers want to include nsGenericHTMLElement.h.
https://hg.mozilla.org/mozilla-central/rev/f3165dffa51c
https://hg.mozilla.org/mozilla-central/rev/b17e0eb827c0
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment on attachment 740656 [details] [diff] [review]
GetDir on root elelement

Yes, perfect.  Thank you!

And yes, GetHtmlElement always returns an <html:html> or null, so this is safe.
Attachment #740656 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 28

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c44612d9cea
https://hg.mozilla.org/mozilla-central/rev/7c44612d9cea

Comment 30

4 years ago
needs documentation at least here
 https://developer.mozilla.org/en-US/docs/DOM/Document.dir
and probably elsewhere
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.