[Webkit-unassigned] [Bug 58505] [Chromium]UI polishes and tweaks to Autofill dropdown menu.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=58505





--- Comment #5 from Naoki Takano <takano.naoki at gmail.com>  2011-04-14 10:53:52 PST ---
(In reply to comment #4)
> (From update of attachment 89517 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89517&action=review
> 
> I know that a lot of the code in this file is shared by the <select> popup (except on Mac) and by the Autofill popup.  I believe we only want these changes to affect the Autofill popup.  Have you verified that this is indeed the case?
I verified them, but I assumed the other popups also need to obey this rule. I'll leave the other popups as it is.

My plan is
1, Add whole padding and margin information into PopupMenuStyle.
2, Add popup window type into PopupMenuStyle and according to the type, change the margin.

I prefer to 1.

> > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:957
> > +        gc->fillRect(separatorRect, Color(0xcd, 0xcd, 0xcd), ColorSpaceDeviceRGB);
> 
> nit: I wonder if there's an existing named color for this.  It seems a little weird to declare a color constant as a function parameter.  I'm not completely up to speed on WebKit style guidelines, though.
There is not #cdcdcd color in Webkit as defined. But as I suggested in bug track comment, I will use #c0c0c0 which is Color::lightGray. That would be better.

> > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1146
> > +        return separatorHeight;
> 
> nit: As mentioned in the Chromium bug, we might want a few pixels of padding around the separator.  I'd need to see more screenshots or play with the code to have a beter idea of what exactly we want here, though.

As Roma isn't worried about it, so I will leave here.

Thanks,

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.