nsOutlinerBodyFrame.cpp need to be bidi enable

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
18 years ago
11 years ago

People

(Reporter: ftang, Assigned: smontagu)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

18 years ago
Hyatt told me that we also need to bidi enable the new outliner widget
I believe the code is in ayout/xul/base/src/outliner/src/nsOutlinerBodyFrame.cpp 
especially 2089     aRenderingContext.DrawString(realText, textRect.x, 
textRect.y);

Comment 1

18 years ago
Switching qa contact to mahar@eg.ibm.com.
QA Contact: andreasb → mahar
Reporter

Comment 2

18 years ago
simon: I believe we use nsOutlinerBodyFrame a lot. (for example, the URL auto
complete), can you fix this one?
Reporter

Comment 3

18 years ago
move component
Component: Internationalization → BiDi Hebrew & Arabic
Assignee

Comment 4

18 years ago
Posted patch Suggested patch (obsolete) — Splinter Review

Comment 5

18 years ago
I'm concerned about the need to iterate over the characters every time to
determine whether or not bidi is enabled.  It also seems that once you've
determined whether or not bidi is enabled for a given outliner, you don't need
to keep checking the characters of the cell text.

Perhaps the question of whether or not Bidi is enabled should be answered by the
view attached to the outliner?
Assignee

Comment 6

18 years ago
>Perhaps the question of whether or not Bidi is enabled should be answered by the
>view attached to the outliner?

That sounds reasonable, but I can't work out how and where to implement it. Can
you give me some pointers?


Mass-move all BiDi Hebrew and Arabic qa to me, zach@zachlipton.com. 
Thank you Gilad for your service to this component, and best of luck to you 
in the future.

Sholom. 
QA Contact: mahar → zach
Assignee

Comment 8

18 years ago
Divide and rule time... This is a patch to make nsOutlinerBodyFrame.cpp display
Bidi text correctly, assuming that the BidiEnabled flag has been set in the
PresContext's mDocument.

What is still lacking is a technique to set that flag, similar to what happens
now in nsGenericDOMDataNode.cpp
Assignee

Updated

18 years ago
Attachment #43995 - Attachment is obsolete: true
Reporter

Updated

18 years ago
Blocks: 115713
Assignee

Comment 9

18 years ago
*** Bug 119894 has been marked as a duplicate of this bug. ***
Assignee

Comment 10

18 years ago
*** Bug 125174 has been marked as a duplicate of this bug. ***
Assignee

Updated

18 years ago
OS: Windows NT → All
Hardware: PC → All
Assignee

Comment 11

18 years ago
*** Bug 125966 has been marked as a duplicate of this bug. ***
Assignee

Updated

18 years ago
Keywords: nsbeta1
Reporter

Comment 12

18 years ago
nsbeta1+, please measure the performance with ji
Keywords: nsbeta1nsbeta1+
Assignee

Comment 13

18 years ago
Maybe it would be no big deal to go through the Bidi path in all cases, since
the Bidi rendering is well optimized for cases which need no special treatment.
I just tested a patch that does that, displaying a full bookmark sidebar with
MOZ_TIMELINE.

The text drawing routine in nsOutlineBodyFrame::PaintText was called 131 times.
With no bidi, this took 0.189 seconds on average, and with bidi 0.194 seconds.
This translates to an increase of less than 0.1% of the whole startup.
Assignee

Comment 14

18 years ago
Posted patch Testing patchSplinter Review
This is the patch referred to in comment 13
Assignee

Comment 15

18 years ago
mkaply: if you agree that attachment 72717 [details] [diff] [review] is the least bad way to do this, can
you r= ?
Assignee

Updated

18 years ago
Status: NEW → ASSIGNED
Comment on attachment 72717 [details] [diff] [review]
Testing patch

r=mkaply
Attachment #72717 - Flags: review+

Comment 17

18 years ago
sr=hyatt
Comment on attachment 72717 [details] [diff] [review]
Testing patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72717 - Flags: approval+
Assignee

Comment 19

18 years ago
Patch checked in
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED

Updated

11 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.