867936 - Remove remaining uses of bidi.direction pref

Status: NEW

(Core :: Layout: Text and Fonts, defect)

Description

[Simon Montagu :smontagu]

Since bug 151407 the pref bidi.direction is no longer used to determine the default direction of web pages. It has some other uses that in principle can be removed as well. Bug 454420 is I think the only case that needs a decision and resolution first.

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

Assuming that bug 454420 will remove the use of the pref for determining scrollbar side, this removes the other uses and the pref itself, also the "bidi.support" pref, which is a fossil that I don't think we have ever had code support for.

FTR, I understand that the "bidi.texttype" pref is used in some terminal emulation webapps so I don't want to remove that.

Comment 2

[(no longer active)]

Comment on attachment 746804 [details] [diff] [review]
Patch

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

Nice!

::: docshell/base/nsIMarkupDocumentViewer.idl
@@ +103,3 @@
>     * Use this attribute to access all the Bidi options in one operation
>     */
> +  attribute octet bidiOptions;

Please change the uuid of this interface.

::: layout/base/nsBidiPresUtils.cpp
@@ +1662,5 @@
>          HandleNumbers(aText,aTextLength,IBMBIDI_NUMERAL_ARABIC);
>        break;
>  
>      case IBMBIDI_NUMERAL_PERSIANCONTEXT:
> +      if ( ( (aIsOddLevel) && (IS_ARABIC_DIGIT (aText[0])) ) || (eCharType_ArabicNumber == aCharType) )

Oh, looking at this brings back "good" memories of when I looked at this code before!

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 746804 [details] [diff] [review]
Patch

Why aren't all bidi-related methods removed from nsDocumentViewer.cpp? I didn't see callers for any of those methods in m-c or c-c.

Comment 5

[Jonathan Kew [:jfkthame]]

(In reply to Simon Montagu :smontagu from comment #1)
> FTR, I understand that the "bidi.texttype" pref is used in some terminal
> emulation webapps so I don't want to remove that.

In what way do webapps use this? Does it have any effect whatsoever on Gecko behavior? I suppose it did in the past, but I suspect that's no longer true.

Comment 6

[Simon Montagu :smontagu]

Those are all interesting questions but for the time being they are all theoretical. If somebody really wants to add value to this bug it would be more useful to get some traction on 454420.

Comment 7

[Simon Montagu :smontagu]

(In reply to Henri Sivonen (:hsivonen) from comment #4)
> Why aren't all bidi-related methods removed from nsDocumentViewer.cpp? I
> didn't see callers for any of those methods in m-c or c-c.

I thought that if it implemented nsIMarkupDocumentViewer it had to retain getters and setters for all the attributes on the interface. Is that not the case?

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to Simon Montagu :smontagu from comment #7)
> I thought that if it implemented nsIMarkupDocumentViewer it had to retain
> getters and setters for all the attributes on the interface. Is that not the
> case?

Not the case if we remove the attributes from the interface. And why wouldn't we?

(In reply to Simon Montagu :smontagu from comment #6)
> Those are all interesting questions but for the time being they are all
> theoretical. If somebody really wants to add value to this bug it would be
> more useful to get some traction on 454420.

If this bug isn't moving, how about we land the patch from bug 946647 to get rid of dead code we could get rid of right away?

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.