On Chromium (Windows and Linux) the following is currently misrendered: <select> <optgroup label="Should be bold"> <option>Should not be bold</option> </optgroup> </select> This occurs because the renderStyle of the element is used as the base style for rendering. In html4.css we have: optgroup { font-weight: bolder; } option { font-weight: normal; } However, in themeWin.css: /* Options always use the selects' fonts */ option { font: inherit !important; } This stops the font-weight: normal in html4.css from taking effect. The overriding rule was added in https://bugs.webkit.org/show_bug.cgi?id=24762 because otherwise a differnt font on the <option> could make the text wider than the <select>. This isn't an issue for the <optgroup> as they can't be selected. Because of this we switch Chromium from using the element's style to using the menuStyle() and implementing the bolding in code. platform/mac/PopupMenuMac.mm: uses the itemStyle. However, themeWin.css isn't in play here so everything works out. platform/gtk/PopupMenuGtk.cpp: renders using GTK. Doesn't appear to observe the style or type save for |itemIsEnabled|. platform/win/PopupMenuWin.cpp: uses the menuStyle and handles bold itself. platform/chromium/PopupMenuChromium.cpp: changed in this patch to observe menuStyle() platform/wx/PopupMenuWx.cpp: renders with WX. Doesn't appear to observe the style or type. platform/qt/PopupMenuQt.cpp: code to observe the style is #if 0'ed out
Created attachment 29550 [details] patch
Comment on attachment 29550 [details] patch The code change is fine. I don't think those comments belong in html4.css though. If anything, we need a comment in themeWin.css about why we're forcing the font to be inherited. Something like "Option font must be inherited because we depend on computing the size of the <select> based on the size of the options, and they must use the same font for that computation to be correct". I hesitate to say anything about the Changelog, because I think we all usually say too little in the logs, but I think you can probably have a more concise explanation here. This is really an r+, but I'd like you to edit the comments & ChangeLog before you checkin. If you do that, pretty much anyone can give you the final r+. Nice work!
Created attachment 29553 [details] patch (Addressing adele's comments)
Created attachment 29555 [details] patch (missed bug link in the ChangeLog)
Comment on attachment 29555 [details] patch r+ based on adele's previous comments.
Landed as: http://trac.webkit.org/changeset/42597