Bug 8408

Summary: Paint the highlight behind selected list markers
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: VERIFIED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Patch
none
Revised patch darin: review+

Description mitz 2006-04-15 07:09:49 PDT
Make non-image list markers inside the selection look like other highlighted text, with the selection color painted behind them, not blended in front.
Comment 1 mitz 2006-04-15 07:27:01 PDT
Created attachment 7727 [details]
Patch

Mostly lifted from RenderReplaced.
Comment 2 Darin Adler 2006-04-15 18:15:30 PDT
Comment on attachment 7727 [details]
Patch

Why isn't there a check of selectionState() inside the "isErrorImage" code path?

The 153 alpha thing from RenderReplaced is really ugly. I'd like it to be something more like this to make the 60% clear:

    const int selectionColorImageOverlayAlpha = 60 * 255 / 100;

If it was in a header file it could be shared with RenderReplaced.

Is it really correct to set the selection state of the item to match the state of the list marker?
Comment 3 mitz 2006-04-16 08:15:11 PDT
Created attachment 7744 [details]
Revised patch

(In reply to comment #2)
> Why isn't there a check of selectionState() inside the "isErrorImage" code
> path?

My mistake, corrected in this version.

> The 153 alpha thing from RenderReplaced is really ugly. I'd like it to be
> something more like this to make the 60% clear:
> 
>     const int selectionColorImageOverlayAlpha = 60 * 255 / 100;
> 
> If it was in a header file it could be shared with RenderReplaced.

Added in RenderObject.h.

> Is it really correct to set the selection state of the item to match the state
> of the list marker?

Yet another mistake. In this version I replaced the list item with the containing block both in setSelecitonState() and in selectionRect().
Comment 4 Darin Adler 2006-04-16 19:49:30 PDT
Comment on attachment 7744 [details]
Revised patch

In RenderListMarker::selectionRect, I think it's a little strange to compute selectionRight by adding width() to selectionLeft, then later just subtract selectionLeft out. Instead we should either just use width() directly or have a selectionWidth local variable.

Otherwise this looks perfect to me.

r=me with or without that change.