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)
Tracking
()
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)
396 bytes,
text/html
|
Details | |
6.27 KB,
patch
|
smontagu
:
review+
dbaron
:
superreview+
mkaply
:
approval-aviary+
mkaply
:
approval-aviary+
mkaply
:
approval1.7.5+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Comment 4•23 years ago
|
||
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.
Comment 6•22 years ago
|
||
Comment 7•22 years ago
|
||
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
Comment 10•22 years ago
|
||
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.
Comment 11•22 years ago
|
||
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?
Comment 13•21 years ago
|
||
(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
Comment 14•21 years ago
|
||
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.
Comment 15•21 years ago
|
||
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
Comment 16•21 years ago
|
||
(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?
Updated•21 years ago
|
Attachment #150136 -
Flags: review?(smontagu)
Comment 17•21 years ago
|
||
Comment on attachment 150136 [details] [diff] [review] up-to-date patch r=smontagu
Attachment #150136 -
Flags: review?(smontagu) → review+
Comment 18•21 years ago
|
||
(In reply to comment #17) > (From update of attachment 150136 [details] [diff] [review]) > r=smontagu > Now can someone chek it in? :-)
Comment 19•21 years ago
|
||
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+
Comment 21•21 years ago
|
||
Patch checked in. Thank you, Lina!
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 22•21 years ago
|
||
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).
Comment 24•21 years ago
|
||
Test also p element, valid code and link to W3c Validator. This should be the standard on all testcases.
Comment 27•20 years ago
|
||
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 28•20 years ago
|
||
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.
Assignee | ||
Comment 30•20 years ago
|
||
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+
Comment 31•20 years ago
|
||
(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
Comment 32•20 years ago
|
||
(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).
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?
Comment 36•17 years ago
|
||
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.
Description
•