Closed Bug 490283 Opened 11 years ago Closed 11 years ago

Mac OS X bidi implementation is not 64-bit ready

Categories

(Core :: Widget: Cocoa, defect)

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jaas, Assigned: jaas)

References

Details

Attachments

(1 file)

widget/src/cocoa/nsBidiKeyboard.mm

"KLGetCurrentKeyboardLayout" and "KLGetKeyboardLayoutProperty" are not available in 64-bit, we'll need to replace our usage. Looks like the replacements are available on Mac OS X 10.5+ only (Text Input Source Services).
Attached patch fix v1.0Splinter Review
The only consumer of this bidi method is caret drawing, debatable whether or not we should be drawing that UI hint anyway.
Assignee: nobody → joshmoz
Attachment #374840 - Flags: review?(jfkthame)
Comment on attachment 374840 [details] [diff] [review]
fix v1.0

On looking into this further, I think we could provide an implementation of sorts, using TISCopyCurrentKeyboardLayoutInputSource to get an input source reference, and then TISGetInputSourceProperty with the kTISPropertyInputSourceLanguages constant to get the primary language, if any, for the selected layout. We'd then look up the language in a list of known RTL languages. This would be for 10.5+ only, as you noted above.

This will not be a perfect solution, as (a) it will be difficult to come up with a definitive list of RTL languages, and (b) keyboard layouts need not necessarily provide this property anyway. But it could probably work for the most common situations, at least.

However, given the questionable value of the feature, and the limitations of any potential implementation, I think the current patch is fine for now, at least until we reconsider whether we really want the UI hint.
Attachment #374840 - Flags: review?(jfkthame) → review+
Attachment #374840 - Flags: superreview?(roc)
Attachment #374840 - Flags: superreview?(roc) → superreview+
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/efe6d58c089d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.