Bug 15713 - Outside list bullets ignore text-align style
: Outside list bullets ignore text-align style
Status: RESOLVED FIXED
: WebKit
Layout and Rendering
: 523.x (Safari 3)
: All All
: P2 Normal
Assigned To:
: http://www.nick-santos.com/tests/list...
: GoogleBug, HasReduction, InRadar
:
:
  Show dependency treegraph
 
Reported: 2007-10-26 12:07 PST by
Modified: 2011-03-21 07:37 PST (History)


Attachments
Demo file (499 bytes, text/html)
2008-08-27 14:26 PST, Dave Hyatt
no flags Details
Fix the position of outside list bullets (60.04 KB, patch)
2010-08-30 02:22 PST, Renata Hodovan
no flags Review Patch | Details | Formatted Diff | Diff
Fix the position of outside list bullets (62.90 KB, patch)
2010-09-20 05:06 PST, Renata Hodovan
no flags Review Patch | Details | Formatted Diff | Diff
Fix the position of outside list bullets (62.90 KB, patch)
2010-09-20 06:16 PST, Renata Hodovan
darin: review-
Review Patch | Details | Formatted Diff | Diff
Fix the position of outside list bullets (9.08 KB, patch)
2010-10-21 04:07 PST, Renata Hodovan
akling: review-
Review Patch | Details | Formatted Diff | Diff
Fix the position of outside list bullets (11.76 KB, patch)
2010-10-21 15:44 PST, Renata Hodovan
no flags Review Patch | Details | Formatted Diff | Diff
Fix the position of outside list bullets (11.87 KB, patch)
2010-10-22 00:31 PST, Renata Hodovan
no flags Review Patch | Details | Formatted Diff | Diff
Fix the position of outside list bullets (9.16 KB, patch)
2010-10-22 06:42 PST, Renata Hodovan
akling: review-
Review Patch | Details | Formatted Diff | Diff
Patch (1.58 MB, patch)
2010-11-16 12:27 PST, Renata Hodovan
no flags Review Patch | Details | Formatted Diff | Diff
Patch (1.65 MB, patch)
2010-11-19 08:15 PST, Renata Hodovan
commit-queue: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Rollout some png files of r72533 to complete r72599. (671.43 KB, patch)
2011-03-21 05:58 PST, zhenghao@google.com
no flags Review Patch | Details | Formatted Diff | Diff
Change the style of patch. (671.46 KB, patch)
2011-03-21 06:07 PST, zhenghao@google.com
no flags Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2007-10-26 12:07:41 PST
1) Create a div with style='text-align: right'
2) Create an ordered list inside that div.

Expected Result:
List bullets should be aligned right.

Actual Result:
List bullets aligned to the left.

Test here:
http://www.nick-santos.com/tests/list_layout.html
------- Comment #1 From 2007-10-27 02:25:26 PST -------
Inside list bullets work just fine.
------- Comment #2 From 2008-08-11 13:54:20 PST -------
We are getting a lot of complaints from users of Google's editor about this issue.  Would love to see this fixed.
------- Comment #3 From 2008-08-11 14:26:58 PST -------
<rdar://problem/6141574>
------- Comment #4 From 2008-08-27 13:57:21 PST -------
CSS2.1 says "The marker box is outside the principal block box". Isn't this bug requesting a violation of CSS2.1?
------- Comment #5 From 2008-08-27 14:04:18 PST -------
It is.  However every other browser honors text-align on outside list bullets.  CSS 2.1 should arguably be amended.  As things stand, however, our rendering is correct.
------- Comment #6 From 2008-08-27 14:18:00 PST -------
The reason we care about this is for rich text editing.  Using the built in execCommands, lists always looks wrong when you align them.  You can see this in action in the midas demo: http://www.mozilla.org/editor/midasdemo/.

Example 1:
1. Type "foo<enter>bar<enter>baz"
2. Select all and right align
3. Apply bullets
Resulting HTML is: <div style="text-align: right;"><ul><li>foo<br></li><li>bar<br></li><li>baz<br></li></ul></div>

As reported earlier in this bug, this renders with the bullets on the left and the text on the right.

Example 2:
1. Type "foo<enter>bar<enter>baz"
2. Select all, apply bullets
3. Right align
Reulting HTML is: <ul><li style="text-align: right;">foo<br></li><li style="text-align: right;">bar<br></li><li style="text-align: right;">baz<br></li></ul>

In this case, it also renders with bullets on the left and text on the right, so there is no workaround for the user to get their lists to look correct.

Should we file this under the rich text component instead?
------- Comment #7 From 2008-08-27 14:20:02 PST -------
Right.  The issue is that Safari's rendering is actually correct according to CSS2.1.  We can't make a fix without being sure that the spec is going to be changed.  I'll bring this up with the CSS working group.

list-style-position:inside would work around the problem BTW.  I don't know if that is an option though.
------- Comment #8 From 2008-08-27 14:26:32 PST -------
Created an attachment (id=23040) [details]
Demo file

Notice that Firefox makes the marker follow text-align, but it does not make the marker follow text-indent.  This seems inconsistent to me.
------- Comment #9 From 2008-08-27 14:55:01 PST -------
A couple extra data points in favor of modifying the spec.

list-style-position:inside doesn't really provide a great solution for left-aligned lists as it puts the marker to the right of the padding-left, which is not what you want either.

To make the WYSIWYG experience actually match user expectation, you would need list-style-position:inside and to decrease the padding accordingly. Not sure that's the "right" solution though.

To be clear, IE and Opera have the marker follow both text-align and text-indent. And, as Hyatt said, FF has the marker follow only text-align, which I agree seems inconsistent.

There is (mostly) browser consistency here and the IE/Opera implementation make for a considerably more intuitive WYSIWYG experience without rich-text editors needing to do crazy hackery around browser defaults.
------- Comment #10 From 2008-08-27 16:21:18 PST -------
(In reply to comment #8)
> Created an attachment (id=23040) [edit] [details]
> Demo file
> 
> Notice that Firefox makes the marker follow text-align, but it does not make
> the marker follow text-indent.  This seems inconsistent to me.

A weird thing that I noticed about firefox is that if you do
<li><div>text</div></li>
then the marker jumps back outside. However, IE and Opera do not do that.
------- Comment #11 From 2008-10-06 11:59:33 PST -------
It's obvious to me that we should fix this, even if it violates CSS2.1.
------- Comment #12 From 2009-09-14 13:39:52 PST -------
Now that we have a clean overflow model that propagates all the way up the line box tree, I think this bug could be fixed easily.

placeBoxesHorizontally should start putting the list marker in the correct spot, and we should just remove all the RenderListItem hackery.
------- Comment #13 From 2010-08-30 02:22:56 PST -------
Created an attachment (id=65893) [details]
Fix the position of outside list bullets
------- Comment #14 From 2010-09-07 01:20:10 PST -------
Any feedbacks are welcome :)
------- Comment #15 From 2010-09-17 05:54:52 PST -------
(From update of attachment 65893 [details])
> +2010-08-30  Renata Hodovan  <reni@inf.u-szged.hu>

Address should be "reni@inf.u-szeged.hu", no?

> +    int getTextAlignOffset() { return m_textAlignOffset; }

This method should be const.

I don't know this code near well enough to comment on the actual changes. It would help greatly if you'd explain what you're doing and why (in the ChangeLog.)
------- Comment #16 From 2010-09-20 05:06:00 PST -------
Created an attachment (id=68071) [details]
Fix the position of outside list bullets
------- Comment #17 From 2010-09-20 05:07:58 PST -------
(In reply to comment #15)
> (From update of attachment 65893 [details] [details])
> > +2010-08-30  Renata Hodovan  <reni@inf.u-szged.hu>
> 
> Address should be "reni@inf.u-szeged.hu", no?
Yeah, you're right. I misspelled it.

> > +    int getTextAlignOffset() { return m_textAlignOffset; }
> 
> This method should be const.
Okay.

> I don't know this code near well enough to comment on the actual changes. It would help greatly if you'd explain what you're doing and why (in the ChangeLog.)
I simply saved the start position of the current line by outside markers and this value should be added to the marker's position.
This patch caused one new failed layout test, but it's expected is updated now.
------- Comment #18 From 2010-09-20 05:13:14 PST -------
Attachment 68071 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4098004
------- Comment #19 From 2010-09-20 06:16:04 PST -------
Created an attachment (id=68074) [details]
Fix the position of outside list bullets
------- Comment #20 From 2010-09-23 09:15:02 PST -------
(From update of attachment 68074 [details])
This looks fairly good to me, I'm not comfortable r+'ing though. Hopefully someone in CC can have a look.

One question: Are you sure m_textAlignOffset can't be used uninitialized in any way? IOW, will InlineFlowBox::placeBoxesHorizontally() always be called before RenderListItem::positionListMarker()?
------- Comment #21 From 2010-10-13 00:19:30 PST -------
(In reply to comment #20)
> (From update of attachment 68074 [details] [details])
> This looks fairly good to me, I'm not comfortable r+'ing though. Hopefully someone in CC can have a look.
> 
> One question: Are you sure m_textAlignOffset can't be used uninitialized in any way? IOW, will InlineFlowBox::placeBoxesHorizontally() always be called before RenderListItem::positionListMarker()?

I'm not sure but it seems to me that the placeBoxesHorizontally() calculates the outer dimensions of the boxes, so it should run earlier than positionListMarker().

Hyatt? What is your opinion? :)
------- Comment #22 From 2010-10-13 18:14:57 PST -------
(From update of attachment 68074 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=68074&action=review

Thanks for taking this on.

This change needs to come with a new test case. Simply updating the results of test cases that were intended to test other things is not sufficiently clear in this case.

It’s considered inelegant to have data hanging off an object that’s only used by code outside the class, and this is the case with m_textAlignOffset.

I also think we could come up with a better name for this concept than “text align offset”. What is this value? An offset for aligning text? I’m not sure that’s the best way to describe it.

I’d like to see code that would help us notice and debug if this value was used without ever being set since the flow of control is indirect. Perhaps a special uninitialized value and an assertion in the getter would do the trick.

review- because of insufficient test cases

> WebCore/rendering/InlineFlowBox.cpp:346
> +                RenderListMarker* currMarker = toRenderListMarker(curr->renderer());
> +                currMarker->setTextAlignOffset(xPos);

The local variable is no help here. It would be clearer to have it on one line.

> WebCore/rendering/RenderListMarker.h:47
> +    int getTextAlignOffset() const { return m_textAlignOffset; }

In the WebKit project we don’t use get in the names of getter functions such as this one. It should be named textAlignOffset.

> WebCore/rendering/RenderListMarker.h:84
> +    int m_textAlignOffset;

The set-up here seems fragile. This is not initialized, so the code will only work correctly if InlineFlowBox::placeBoxesHorizontally is called before positionListMarker. Is this guaranteed?
------- Comment #23 From 2010-10-21 04:07:55 PST -------
Created an attachment (id=71420) [details]
Fix the position of outside list bullets
------- Comment #24 From 2010-10-21 04:51:52 PST -------
> This change needs to come with a new test case. Simply updating the results of test cases that were intended to test other things is not sufficiently clear in this case.
Added.

> It’s considered inelegant to have data hanging off an object that’s only used by code outside the class, and this is the case with m_textAlignOffset.
> 
> I also think we could come up with a better name for this concept than “text align offset”. What is this value? An offset for aligning text? I’m not sure that’s the best way to describe it.
You're right, it was really not too correct. I renamed it to outsideMarkerOffset.

> I’d like to see code that would help us notice and debug if this value was used without ever being set since the flow of control is indirect. Perhaps a special uninitialized value and an assertion in the getter would do the trick.
It's done. First outsideMarkerOffsets value is INT_MAX and an assertion checks it in getter.
------- Comment #25 From 2010-10-21 04:53:56 PST -------
(From update of attachment 71420 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=71420&action=review

> WebCore/ChangeLog:8
> +        Fix the outside markers position with saving the start position of the current line and adding this value to the markers postion.

s/markers/marker's/g
s/with/by/

> WebCore/rendering/RenderListMarker.h:48
> +    const int outsideMarkerOffset() const 

Unnecessary const, should be "int outsideMarkerOffset() const"

> LayoutTests/ChangeLog:5
> +        Need a short description and bug URL (OOPS!)

Oops, indeed!

> LayoutTests/fast/lists/outSideListMarkers.html:20
> +<p><u>text-align: rigth</u></p>

s/rigth/right/
------- Comment #26 From 2010-10-21 04:55:29 PST -------
Also, doesn't this still affect editing/pasteboard/paste-4039777-fix.html? A new baseline is missing from the latest patch.
------- Comment #27 From 2010-10-21 05:13:21 PST -------
Attachment 71420 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4690011
------- Comment #28 From 2010-10-21 15:44:49 PST -------
Created an attachment (id=71504) [details]
Fix the position of outside list bullets
------- Comment #29 From 2010-10-21 15:47:23 PST -------
(In reply to comment #26)
> Also, doesn't this still affect editing/pasteboard/paste-4039777-fix.html? A new baseline is missing from the latest patch.
Yeah, it was missing :$ Sorry...
------- Comment #30 From 2010-10-21 16:26:49 PST -------
Attachment 71504 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4659015
------- Comment #31 From 2010-10-22 00:31:15 PST -------
Created an attachment (id=71531) [details]
Fix the position of outside list bullets
------- Comment #32 From 2010-10-22 06:42:36 PST -------
Created an attachment (id=71562) [details]
Fix the position of outside list bullets
------- Comment #33 From 2010-11-01 16:51:37 PST -------
(From update of attachment 71562 [details])
I took this patch for a spin, and it broke the following tests on Snow Leopard:

css1/box_properties/border_left.html
css1/box_properties/margin_right.html
css1/box_properties/padding_left.html
css1/box_properties/padding_right.html
css2.1/t0805-c5521-brdr-l-02-e.html
editing/selection/extend-by-word-002.html
fast/css/background-shorthand-invalid-url.html
fast/css/empty-pseudo-class.html
fast/css/first-child-pseudo-class.html
fast/css/first-of-type-pseudo-class.html
fast/css/last-child-pseudo-class.html
fast/css/last-of-type-pseudo-class.html
fast/css/list-outline.html
fast/css/only-child-pseudo-class.html
fast/css/only-of-type-pseudo-class.html
fast/lists/001.html
fast/lists/002.html
fast/lists/003.html
fast/lists/007.html
fast/lists/008.html
fast/lists/ordered-list-with-no-ol-tag.html

Some of them are definitely progressions, but please look through them and rebaseline as appropriate in the next patch iteration.

I uploaded the results here for your convenience:
http://chaos.troll.no/~kling/15713/layout-test-results/results.html
------- Comment #34 From 2010-11-16 12:27:21 PST -------
Created an attachment (id=74023) [details]
Patch
------- Comment #35 From 2010-11-17 02:07:17 PST -------
(From update of attachment 74023 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=74023&action=review

You asked me to look at this, so here are some (somewhat superficial) comments.
Overall I think you need to get better at explaining your changes.

> WebCore/ChangeLog:9
> +        Fix the outside marker's position by setting a suitable value to its logicalLeft member. For handling embedded markers correctly
> +        a loop is added which determines these marker's width. Some variables become unnecessary and they are removed from the code.

Setting it to "a suitable value" is too unspecific.
You can leave out the sentence about unnecessary variables.

> WebCore/rendering/InlineFlowBox.cpp:276
> +        if (curr->renderer()->isListMarker() && !toRenderListMarker(curr->renderer())->isInside())
> +            markerWidth += toRenderListMarker(curr->renderer())->width() - toRenderListMarker(curr->renderer())->marginLeft();

IMO this would be clearer like so:

if (!curr->renderer()->isListMarker())
    continue;
RenderListMarker* marker = toRenderListMarker(curr->renderer());
if (marker->isInside())
    markerWidth += marker->width() - marker->marginLeft();

> WebCore/rendering/InlineFlowBox.cpp:356
> +                markerWidth -= (toRenderListMarker(curr->renderer())->width() - toRenderListMarker(curr->renderer())->marginLeft());

Same as above:

RenderListMarker* marker = toRenderListMarker(curr->renderer());
markerWidth -= marker->width() - marker->marginLeft();

> WebCore/rendering/RenderListItem.cpp:267
> +            if (!style()->isHorizontalWritingMode())
> +                markerLogicalLeft += paddingStart();

I don't understand this part.
------- Comment #36 From 2010-11-19 08:15:43 PST -------
Created an attachment (id=74391) [details]
Patch
------- Comment #37 From 2010-11-19 08:23:05 PST -------
(From update of attachment 74391 [details])
r=me
------- Comment #38 From 2010-11-22 03:46:22 PST -------
(From update of attachment 74391 [details])
Rejecting patch 74391 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'apply-attachment', '--force-clean', '--non-interactive', 74391]" exit_code: 2
Last 500 characters of output:
t/lists/007-vertical-expected.checksum
patching file LayoutTests/platform/mac/fast/lists/007-vertical-expected.txt
patching file LayoutTests/platform/mac/fast/lists/008-vertical-expected.checksum
patching file LayoutTests/platform/mac/fast/lists/008-vertical-expected.txt
patching file LayoutTests/platform/mac/fast/lists/outSideListMarkers-expected.checksum

Failed to run "[u'/Users/abarth/git/webkit-queue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Andreas Kling', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/6223111
------- Comment #39 From 2010-11-22 10:04:41 PST -------
This is landed in <http://trac.webkit.org/changeset/72527>
Closing bug.
------- Comment #40 From 2010-11-22 21:50:00 PST -------
Reopening.

This patch had a few problems:

(1) It broke vertical outside list bullets.  I'm not sure why you thought the math for vertical bullets had to be different.  It should be identical to horizontal.

(2) It broke nested blocks inside list items when the blocks had borders/padding.  You can't just discard lineOffset.  Even respecting text-align, this was breakage that caused us to not match other browsers.

(3) It clearly violates the CSS2.1 Candidate Recommendation.  Now I am sympathetic to the compatibility argument, but let's see if we can get some traction in the Working Group.  I'm very reluctant (in standards mode at least... quirks mode is another story) to just deliberately check in a patch that contradicts the CSS2.1 text.

I sent the following email to try to get discussion going on this:

http://lists.w3.org/Archives/Public/www-style/2010Nov/0355.html
------- Comment #41 From 2010-11-23 00:57:05 PST -------
Original commits -- rollout commits
------------------------------------
http://trac.webkit.org/changeset/72527 - http://trac.webkit.org/changeset/72582
http://trac.webkit.org/changeset/72529 - http://trac.webkit.org/changeset/72583
http://trac.webkit.org/changeset/72530 - http://trac.webkit.org/changeset/72589
http://trac.webkit.org/changeset/72533 - not rolled out yet!!!
http://trac.webkit.org/changeset/72544 - http://trac.webkit.org/changeset/72589


I think http://trac.webkit.org/changeset/72533 should be rolled out too,because http://trac.webkit.org/changeset/72527 was rolled out. But I'm not sure if
the whole r72533 is related to r72527. Anton, could you check it, please?
------- Comment #42 From 2010-11-23 01:27:28 PST -------
http://trac.webkit.org/changeset/72589 might have broken Leopard Intel Release (Tests)
The following tests are not passing:
inspector/styles-source-lines-inline.html
------- Comment #43 From 2010-11-23 05:21:13 PST -------
(In reply to comment #41)
> Original commits -- rollout commits
> ------------------------------------
> http://trac.webkit.org/changeset/72527 - http://trac.webkit.org/changeset/72582
> http://trac.webkit.org/changeset/72529 - http://trac.webkit.org/changeset/72583
> http://trac.webkit.org/changeset/72530 - http://trac.webkit.org/changeset/72589
> http://trac.webkit.org/changeset/72533 - not rolled out yet!!!
> http://trac.webkit.org/changeset/72544 - http://trac.webkit.org/changeset/72589
> 
> 
> I think http://trac.webkit.org/changeset/72533 should be rolled out too,because http://trac.webkit.org/changeset/72527 was rolled out. But I'm not sure if
> the whole r72533 is related to r72527. Anton, could you check it, please?

Csaba, thanks a lot for adding into the loop.  llya (loislo@chromium.org) and me are rolling rebaselines out.
------- Comment #44 From 2011-03-03 00:25:20 PST -------
Anton, does this http://trac.webkit.org/changeset/72599 change rollout ttp://trac.webkit.org/changeset/72533 ? I think it's incomplete, though.

It only rollouted the checksum files, while all png files were left modified. This could make layout test pass, because when we run these tests on chromium,
as long as the checksum matches, they pass without generate image diff. So the problem is not visible on chromium now.
(In reply to comment #43)
> (In reply to comment #41)
> > Original commits -- rollout commits
> > ------------------------------------
> > http://trac.webkit.org/changeset/72527 - http://trac.webkit.org/changeset/72582
> > http://trac.webkit.org/changeset/72529 - http://trac.webkit.org/changeset/72583
> > http://trac.webkit.org/changeset/72530 - http://trac.webkit.org/changeset/72589
> > http://trac.webkit.org/changeset/72533 - not rolled out yet!!!
> > http://trac.webkit.org/changeset/72544 - http://trac.webkit.org/changeset/72589
> > 
> > 
> > I think http://trac.webkit.org/changeset/72533 should be rolled out too,because http://trac.webkit.org/changeset/72527 was rolled out. But I'm not sure if
> > the whole r72533 is related to r72527. Anton, could you check it, please?
> 
> Csaba, thanks a lot for adding into the loop.  llya (loislo@chromium.org) and me are rolling rebaselines out.
------- Comment #45 From 2011-03-03 05:51:50 PST -------
That pretty much should be the case.  In any event I used standard rebaseline tool, so this patch is to recreate at any convenient moment.

(In reply to comment #44)
> Anton, does this http://trac.webkit.org/changeset/72599 change rollout ttp://trac.webkit.org/changeset/72533 ? I think it's incomplete, though.
> 
> It only rollouted the checksum files, while all png files were left modified. This could make layout test pass, because when we run these tests on chromium,
> as long as the checksum matches, they pass without generate image diff. So the problem is not visible on chromium now.
> (In reply to comment #43)
> > (In reply to comment #41)
> > > Original commits -- rollout commits
> > > ------------------------------------
> > > http://trac.webkit.org/changeset/72527 - http://trac.webkit.org/changeset/72582
> > > http://trac.webkit.org/changeset/72529 - http://trac.webkit.org/changeset/72583
> > > http://trac.webkit.org/changeset/72530 - http://trac.webkit.org/changeset/72589
> > > http://trac.webkit.org/changeset/72533 - not rolled out yet!!!
> > > http://trac.webkit.org/changeset/72544 - http://trac.webkit.org/changeset/72589
> > > 
> > > 
> > > I think http://trac.webkit.org/changeset/72533 should be rolled out too,because http://trac.webkit.org/changeset/72527 was rolled out. But I'm not sure if
> > > the whole r72533 is related to r72527. Anton, could you check it, please?
> > 
> > Csaba, thanks a lot for adding into the loop.  llya (loislo@chromium.org) and me are rolling rebaselines out.
------- Comment #46 From 2011-03-21 05:58:37 PST -------
Created an attachment (id=86310) [details]
Rollout some png files of r72533 to complete r72599.
------- Comment #47 From 2011-03-21 06:01:25 PST -------
Attachment 86310 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

LayoutTests/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #48 From 2011-03-21 06:07:36 PST -------
Created an attachment (id=86312) [details]
Change the style of patch.
------- Comment #49 From 2011-03-21 07:36:58 PST -------
(From update of attachment 86312 [details])
Clearing flags on attachment: 86312

Committed r81579: <http://trac.webkit.org/changeset/81579>
------- Comment #50 From 2011-03-21 07:37:17 PST -------
All reviewed patches have been landed.  Closing bug.