Don't set NS_FRAME_IS_BIDI too enthusiastically

NEW
Assigned to

Status

()

Core
Layout: Text
12 years ago
10 years ago

People

(Reporter: smontagu, Assigned: smontagu)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

12 years ago
Spun off from bug 333433.

------- Comment #4 From Uri Bernstein 2006-04-10 08:19 PDT [reply] -------
On a side note, I question the usefulness of the NS_FRAME_IS_BIDI flag, as in
all cases I know of, it seems to be set for all the frames on a page which
contains any RTL at all.

------- Comment #7 From Simon Montagu  2006-04-10 09:27 PDT  [reply] -------
It shouldn't be being set on frames in an LTR-only paragraph even if other
paragraphs contain RTL. Is it?

------- Comment #8 From Uri Bernstein 2006-04-10 09:51 PDT [reply] -------
I double-checked, and it is. Resolve() is called on the LTR-only paragraph
(should it be?), and it calls AdjustOffsetsForBidi(), which sets the flag.

------- Comment #9 From Simon Montagu  2006-04-10 10:01 PDT  [reply] -------
That seems wrong to me. I think EnsureBidiContinuation() should be setting it.

------- Comment #11 From Uri Bernstein 2006-04-10 10:21 PDT [reply] -------
I tracked this back to the patch on bug 74946, which added setting the flag on
both branches of the |if ( (runLength > 0) && (runLength < fragmentLength) )|.
As far as I can tell, the behaviour didn't change ever since that patch was
landed.

------- Comment #12 From Simon Montagu 2006-04-10 11:26 PDT [reply] -------
I still think it's wrong :) The bit was originally supposed to mean "this frame
either is a nextBidi or has a nextBidi"[1], though it is overloaded for
TextBoxFrames to mean "this frame has some RTL content". Let's continue this
discussion in a new bug.

[1]http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsIFrame.h&rev=3.316&mark=213-214#211
(Assignee)

Comment 1

12 years ago
Another case where (IMO) the bit shouldn't be set is something like:

<span>foo</span><span>bar BAZ</span>

(where BAZ represents RTL text)

There shouldn't be any reason for foo to have NS_FRAME_IS_BIDI set.

I still thing we should consider whether we need this bit at all.
Suppose we get rid of it (and behave as if it's set whenever the document has RTL). The overall behaviour will be identical to what we have today. What would we be losing (compared to setting the flag only when necessary)? Performance? Is it significant?
(Assignee)

Comment 3

12 years ago
I think there is a potential performance win in only setting the bit when it should be set, e.g. on pages that are mostly LTR with one or two words in RTL. This typically occurs on pages with links to all the different localized versions of the same content. Wikipedia pages are an obvious example.

Updated

11 years ago
Depends on: 351650

Updated

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