Closed Bug 140611 Opened 23 years ago Closed 21 years ago

When a block element is first in RTL <LI>, bullet moves left

Categories

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

Product:
Core
Component:
Layout: Text and Fonts
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: blanchardmatthew, Assigned: mkaply)

References

(Blocks 1 open bug)

Details

(Keywords: fixed-aviary1.0, fixed1.7, rtl)

Attachments

(3 files, 2 obsolete files)

HTML sample: bullet/number incorrectly moves left
396 bytes, text/html
Details
up-to-date patch
6.27 KB, patch
smontagu
: review+
dbaron
: superreview+
mkaply
: approval-aviary+
mkaply
: approval-aviary+
mkaply
: approval1.7.5+
Details | Diff | Splinter Review
Better testcase
975 bytes, text/html
Details
From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.9+) Gecko/20020329 BuildID: 20020329 When a block element appears immediately after <LI> in RTL lists, the bullet (or the number) of the item appears on the left, not on the right. Reproducible: Always Steps to Reproduce: <ul dir="rtl"> <li>one <li>two <li><div>three</div> <li>four </ul> The block element must be the first node inside the LI element for this bug to occur. If there's some inline element or some text preceding the block element, the bullet (or the number) appears correctly -- on the right.
Confirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 137995
*** Bug 141009 has been marked as a duplicate of this bug. ***
BTW, I think it is important to fix this bug because Microsoft's Outlook Express adds DIVs to lists, and consequently Messenger users that receive HTML email from OE users will sometimes have the bullets/numbers on the left.
*** Bug 204232 has been marked as a duplicate of this bug. ***
Attached patch proposed patch (obsolete) — — Splinter Review
Disnatce a marker from the right border edge of the associated principal box. The fix is supposed to cover also bug 202471 and, partly, bug 206089.
Attachment #124687 - Attachment is obsolete: true
*** Bug 208248 has been marked as a duplicate of this bug. ***
isn't bug 74880 a consequence of this bug ?
In my opinion, bug 208248 is a duplicate of bug 74880 (and not of the current one), and both of them (bug 208248 and bug 74880) are unrelated to the current bug.
The proposed fix (attachment 124734 [details] [diff] [review]) is based on CSS2 spec. CSS3 is going to introduce a change from CSS2: "Markers are now aligned relative to the line box edge, rather than the border edge". How we proceed?
Seams to be fixed from 1.7a :-)
(In reply to comment #12) > Seams to be fixed from 1.7a :-) Oh sorry this hasn't benn fixed at all. I confused with bug 198869
Blocks: 240501
Lina Kemmel, could you please update your patch and ask for reviews? It's a shame that your hard work gets bit-rotted instead of checked in (Bug 195909 comes to mind). Thanks, Prog.
Attached patch up-to-date patch — — Splinter Review
In addition to said in comment #7, the patch removes some obsolete (after D.Baron's fixes for bug 74880) code, and contains css changes (replacing marker's 'margin-right' with '-moz-margin-end').
Attachment #124734 - Attachment is obsolete: true
(In reply to comment #15) > Created an attachment (id=150136) > up-to-date patch > > In addition to said in comment #7, the patch removes some obsolete (after > D.Baron's fixes for bug 74880) code, and contains css changes (replacing > marker's 'margin-right' with '-moz-margin-end'). > Lina, can you please ask for review?
Attachment #150136 - Flags: review?(smontagu)
Comment on attachment 150136 [details] [diff] [review] up-to-date patch r=smontagu
Attachment #150136 - Flags: review?(smontagu) → review+
(In reply to comment #17) > (From update of attachment 150136 [details] [diff] [review]) > r=smontagu > Now can someone chek it in? :-)
Comment on attachment 150136 [details] [diff] [review] up-to-date patch (In reply to comment #18) > Now can someone chek it in? :-) An sr= is needed first. Prog.
Attachment #150136 - Flags: superreview?(dbaron)
Comment on attachment 150136 [details] [diff] [review] up-to-date patch I don't understand the nsLineLayout changes, but sr=dbaron.
Attachment #150136 - Flags: superreview?(dbaron) → superreview+
Patch checked in. Thank you, Lina!
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Simon, David, thanks a lot for reviewing and checking in the changes. (In reply to comment #20) nsLineLayout changes: - remove unnecessary adjustment of widths by |aLineBounds.x|, - in visual RTL, reposition the frames also if |dx == 0|, - do not adjust a bullet, since |nsBlockFrame::ReflowBullet| already did the job. ---------------- One question please: Should aligning of a list item horizontally affect the outside marker? E.g. is a LTR LI with various alignments supposed to be displayed as: (|*| stays for the marker) ----------------------------------------------- |*| a left-aligned item | ----------------------------------------------- ----------------------------------------------- |*| a centered item | ----------------------------------------------- ----------------------------------------------- |*| a right-aligned item | ----------------------------------------------- - or should it adhere to the content position?) (I think content aligning shouldn't affect both the principal block box and the marker box dimensions or position, i.e. the marker should not be repositioned.) Depending on your answer, a LTR or RTL LI will need a small fix (but I'll file a new bug for it).
any chance to checkk this into aviary too?
Attached file Better testcase —
Test also p element, valid code and link to W3c Validator. This should be the standard on all testcases.
*** Bug 252499 has been marked as a duplicate of this bug. ***
*** Bug 253717 has been marked as a duplicate of this bug. ***
Comment on attachment 150136 [details] [diff] [review] up-to-date patch Asking for approval-aviary, since 2 dupes have recently been filed against firefox 0.9.1
Attachment #150136 - Flags: approval-aviary?
Comment on attachment 150136 [details] [diff] [review] up-to-date patch Asking for approval-aviary, since 2 dupes have recently been filed against firefox.
(In reply to comment #22) > One question please: > > Should aligning of a list item horizontally affect the outside marker? According to CSS2, no. (Assuming aligning refers to 'text-align' and not to 'auto' margins, in which case the answer is definitely yes.) According to CSS2.1, it's undefined. According to css3-lists (draft), it's no, but that might change.
Comment on attachment 150136 [details] [diff] [review] up-to-date patch a=mkaply Let's put it on 1.7 too
Attachment #150136 - Flags: approval1.7.3+
Attachment #150136 - Flags: approval-aviary?
Attachment #150136 - Flags: approval-aviary+
Keywords: fixed1.7
(In reply to comment #30) > Let's put it on 1.7 too This bug still show in 1.8a2, but fixed nightly build 2004080408 I checked only on Mac OS X 10.3.3
(In reply to comment #28) > Asking for approval-aviary, since 2 dupes have recently been filed against > firefox. As the (not so) proud submitter of one of those dupes, may I ask: your request to check in to aviary was approved a few days ago (together with approving 1.7), yet as far as I can tell, you checked the fix in to 1.7 but not to the Aviary branch. Is there a reason for this, or is it just an oversight? (or did I just read the CVS logs incorrectly? - I'm still learning my way around here).
It was an oversight. Thanks for the reminder.
Keywords: fixed-aviary1.0
verified
Status: RESOLVED → VERIFIED
This caused regression bug 259683. I'm likely to back a small part of the patch out to fix the regression. What do I need to test?
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl
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.

Attachment

General

Creator:
Created:
Updated:
Size: