Bug 31619

Summary:

Whitespaces after a list marker isn't ignored in calcInlinePrefWidths

Product:

WebKit

Reporter:

Shinichiro Hamaji <hamaji>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

mitz

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
test case with ahem	none
Patch v1	none

Shinichiro Hamaji

Reported 2009-11-18 02:01:00 PST

Created attachment 43416 [details] test case with ahem As mitz pointed in Bug 23264, whitespace characters after a list marker is added into the preferred width.

Attachments
test case with ahem (1023 bytes, text/html) 2009-11-18 02:01 PST, Shinichiro Hamaji	no flags	Details
Patch v1 (4.15 KB, patch) 2009-11-18 02:05 PST, Shinichiro Hamaji	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Shinichiro Hamaji

Comment 1 2009-11-18 02:05:36 PST

Created attachment 43417 [details] Patch v1

mitz

Comment 2 2009-11-18 07:12:21 PST

I’m confused. If you fix this bug independently of bug 23264, you will have an inconsistency between calcPrefWidths() and findNextLineBreak(). I think you should change both at the same time.

Shinichiro Hamaji

Comment 3 2009-11-18 07:36:47 PST

It is already inconsistent for normal lists. Bug 23264 only fixes list-style-position:inside case and the rendering of normal lists was OK even before Bug 23264. Here is the summary of cases: 1. rendering of normal list: space is ignored (OK) 2. rendering of inside list: space is not ignored (not OK) 3. prefWidth of normal list: space is not ignored (not OK, inconsistent with case 1) 4. prefWidth of inside list: space is not ignored (not OK) This patch fixes 3 and 4, and Bug 23264 fixes 2. Anyway, if you want to fix them at the same time, I'll combine the patches into one patch.

mitz

Comment 4 2009-11-18 07:44:37 PST

(In reply to comment #3) > 3. prefWidth of normal list: space is not ignored (not OK, inconsistent with > case 1) Do you have a test case showing this? This case seems to behave correctly to me. > This patch fixes 3 and 4, and Bug 23264 fixes 2. Anyway, if you want to fix > them at the same time, I'll combine the patches into one patch. I suggest that you do that.

Shinichiro Hamaji

Comment 5 2009-11-18 07:47:44 PST

Comment on attachment 43417 [details] Patch v1 Clearing review? as I'll combine this patch into Bug 23264

Shinichiro Hamaji

Comment 6 2009-11-18 07:49:11 PST

(In reply to comment #4) > (In reply to comment #3) > > 3. prefWidth of normal list: space is not ignored (not OK, inconsistent with > > case 1) > > Do you have a test case showing this? This case seems to behave correctly to > me. I think the test case I attached in this bug shows this bug. > I suggest that you do that. Thanks for the quick response. I'll do it soon.

mitz

Comment 7 2009-11-18 08:08:22 PST

(In reply to comment #6) > (In reply to comment #4) > > (In reply to comment #3) > > > 3. prefWidth of normal list: space is not ignored (not OK, inconsistent with > > > case 1) > > > > Do you have a test case showing this? This case seems to behave correctly to > > me. > > I think the test case I attached in this bug shows this bug. You’re right! I was testing the wrong thing :)

Shinichiro Hamaji

Comment 8 2009-11-18 08:28:39 PST

> You’re right! I was testing the wrong thing :) Yay, it's a very rare case. When the webkit reviewers disagree with me, almost always I'm wrong :)

Shinichiro Hamaji

Comment 9 2009-11-18 08:36:18 PST

*** This bug has been marked as a duplicate of bug 23264 ***

Note You need to log in before you can comment on or make changes to this bug.