Closed Bug 1019470 Opened 6 years ago Closed 6 years ago

CSS - LI default left padding after bullet disappeared

Categories

(Core :: Layout, defect)

32 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox30 --- unaffected
firefox31 --- unaffected
firefox32 + fixed
firefox33 --- verified

People

(Reporter: lh, Assigned: jfkthame)

References

()

Details

(4 keywords)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release)
Build ID: 20140602030202

Steps to reproduce:

<!DOCTYPE html>
<html>
<head>
<style>
LI {
    list-style-image: url('http://cdn2.artwhere.net/www.cinetelerevue.be/design/bleu.gif');
}
</style>
</head>
<body>
<ul>
    <li>test</li>
</ul>
</body>
</html>


Actual results:

http://screencast.com/t/fQClyQuHF9f


Expected results:

http://screencast.com/t/yIWIsAyP1

This result comes from Firefox 29, Chrome, ...
Keywords: css2
Summary: LI default left padding after bullet disappeared → CSS - LI default left padding after bullet disappeared
I can reproduce this on mac as well.
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
OS: Windows 7 → All
Product: Firefox → Core
Hardware: x86_64 → All
Jonathan, this looks like bug 1013160 - is this change intentional? That would surprise me, but maybe I'm missing something...
Blocks: 1013160
Component: General → Layout
Flags: needinfo?(jfkthame)
This may need to be discussed in CSSWG as they have decided that the spacing only depends on the suffix. So many influences by that spec...
I'd like to add a half em spacing to image bullets like what we have done for graphic bullets.
Version: 32 Branch → Trunk
Something like this should be OK; we may want to raise it in the WG for further discussion, but for now I think we need a simple fix for the visible regression.
Attachment #8433275 - Flags: review?(matspal)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Flags: needinfo?(jfkthame)
Version: Trunk → 32 Branch
Comment on attachment 8433275 [details] [diff] [review]
restore space after list-style-image bullet.

Xidorn, is this the sort of thing you also had in mind here? (Your comments came in while I was rebuilding, so I saw them after writing the patch!)
Attachment #8433275 - Flags: feedback?(quanxunzhen)
Comment on attachment 8433275 [details] [diff] [review]
restore space after list-style-image bullet.

Review of attachment 8433275 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Jonathan Kew (:jfkthame) from comment #7)
> Comment on attachment 8433275 [details] [diff] [review]
> restore space after list-style-image bullet.
> 
> Xidorn, is this the sort of thing you also had in mind here? (Your comments
> came in while I was rebuilding, so I saw them after writing the patch!)

Yes, this patch LGTM.

I just wonder if there is any helper function which makes it convenient to append space to a nsMargin according to the writing mode. I don't really like the duplication here. Anyway, as a temporary fix, it should be good.
Attachment #8433275 - Flags: feedback?(quanxunzhen) → feedback+
(In reply to Xidorn Quan from comment #8)
> I just wonder if there is any helper function which makes it convenient to
> append space to a nsMargin according to the writing mode.

Not at the moment, AFAIK. Maybe we will eventually switch over the mPadding field to be a LogicalMargin, as part of doing more of the work in logical coordinates instead of physical, in which case it'd become trivial.
Comment on attachment 8433275 [details] [diff] [review]
restore space after list-style-image bullet.

>+      // Add spacing to the padding

Nit: please add a full stop to end the sentence.
(also in the other place)

Will you add a reftest for this later?
(perhaps we could extend bullet-space-1.html for this case?)
Attachment #8433275 - Flags: review?(matspal) → review+
I'm curious why we treat both vertical directions the same.
I'd naively think that the bullet padding in bottom-to-top writing mode
would be on the top side. (Or do we not support bottom-to-top?)
Flags: in-testsuite?
(In reply to Mats Palmgren (:mats) from comment #11)
> I'm curious why we treat both vertical directions the same.
> I'd naively think that the bullet padding in bottom-to-top writing mode
> would be on the top side. (Or do we not support bottom-to-top?)

Right, it's not entirely clear how far we should/will support bottom-to-top. The two important vertical cases are both top-to-bottom, with the block direction being either left-to-right or right-to-left.

The additional case would be wm.IsVertical() && !wm.IsBidiLTR(), but I think it may be premature to add code for that at this stage, until we're closer to actually having usable vertical-text support.

(In reply to Mats Palmgren (:mats) from comment #10)
> Will you add a reftest for this later?
> (perhaps we could extend bullet-space-1.html for this case?)

Yes, I was thinking it should be possible to do a similar test here.
Attachment #8433557 - Flags: review?(matspal)
Attachment #8433557 - Flags: review?(matspal) → review+
Actually, I think we can do the comparison more simply and directly in this case. (Sorry about the churn.)
Attachment #8433570 - Flags: review?(matspal)
Attachment #8433557 - Attachment is obsolete: true
Attachment #8433570 - Flags: review?(matspal) → review+
Duplicate of this bug: 1020087
Duplicate of this bug: 1021238
Duplicate of this bug: 1021421
Duplicate of this bug: 1029318
QA Whiteboard: [good first verify]
bug resolved for Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:33.0) Gecko/20100101 Firefox/33.0 buildID	20140716030202

http://imgur.com/oLTSd8w,D10ucap

[bugday-20140716]
QA Whiteboard: [good first verify] → [good first verify] [bugday-20140716]
You need to log in before you can comment on or make changes to this bug.