88100 - Bidi preferences are not hooked up

Status: VERIFIED FIXED

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

Description

[Mike Kaply [:mkaply]]

This is separate from the UI!!!!

We are missing the code in nsPresContext.cpp that hooks up the Bidi prefefences 
to actually do something.

Diff attached.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Hookup Bidi preferences

Comment 2

[Mike Kaply [:mkaply]]

I actually tried setting the preferences to default to RTL instead of LTR and 
noticed some interesting issues, especially in preferences.

We might want to do some investigation into this.

Comment 3

[Simon Montagu :smontagu]

What happens about defaults? should we set mBidi to IBMBIDI_DEFAULT_BIDI_OPTIONS
before all the |GetIntPref| calls, or in the constructor?

Comment 4

[Mike Kaply [:mkaply]]

The defaults will come from all.js if the pref is not set by the user, 
so we should be OK.

Setting mBidi to IBMBIDI_DEFAULT_BIDI_OPTIONS in the constructor might 
be a good thing to do, though, if only for clarity's sake.

Comment 5

[Simon Montagu :smontagu]

>I actually tried setting the preferences to default to RTL instead of LTR and 
>noticed some interesting issues, especially in preferences.

Example? I tried it and it looked as reasonable as Latin text with default RTL
direction ever does, i.e. there were things like

        :Choose your preferred language for Mozilla

This is one of the reasons that we decided that default direction made more
sense as a per-page override than a preference.

Comment 6

[Mike Kaply [:mkaply]]

I saw a couple things:

Home page url looked like this:

/http://www.mozilla.org

and checkboxes were right aligned.

Comment 7

[Frank Tang]

Is this valid? Your code will make every window use the new pref value once
people change the Pref UI. Is that what we WANT to do? I though the Pref UI
should only control the value of a new window. and once the window get created,
we should let the BIDI Menu (after we add it) to control it instead. 

Comment 8

[Mike Kaply [:mkaply]]

I considered this similar to the font preferences. That is when I change the 
font I want to use, it changes it for the page I am currently viewing.

Comment 9

[Zach Lipton [:zach]]

Mass-move all BiDi Hebrew and Arabic qa to me, [e-mail]. 
Thank you Gilad for your service to this component, and best of luck to you 
in the future.

Sholom. 

Comment 10

[Simon Montagu :smontagu]

I agree that a change in prefs should come into effect on all open pages, BUT I
don't think that a dynamic change to the options (from the Bidi menu once we
have it) should persist to a new document, so we should cut out the #ifdef
IBMBIDI sections in docshell/base/nsDocShell.cpp.

Add that and r=[e-mail]

Comment 11

[Mike Kaply [:mkaply]]

Attachment: Remove BIDI code from nsDocShell so we don't inherit Bidi Options

Comment 12

[Christopher Blizzard (:blizzard)]

sr=blizzard on both patches.

Comment 13

[Mike Kaply [:mkaply]]

when you change the preference to RTL, the WHOLE browser goes RTL. Looks pretty 
cool.

still needs some work though.

This bug is fixed.

Comment 14

[Mike Kaply [:mkaply]]

verify. preferences are working if you change them in the js file.

Comment 15

[Shoshannah Forbes]

is there a plan to give usrs a GUI for this prefs?  Where do we have
documatition about what they are and what they do?