Summary: | [WinCairo] <select> elements do not popup options | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Stephan Szabo <stephan.szabo> | ||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, commit-queue, Hironori.Fujii, koivisto, lforschler, rniwa, ysuzuki | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Stephan Szabo
2018-07-30 10:23:08 PDT
Created attachment 346069 [details]
Patch
https://github.com/WebKit/webkit/commit/43768040539a01ec2d52954f17fdb39763724623 This is the revision the original implementaion has been removed. Bug 48701 – Windows Popup widget support (<select>) The original bug ticket. Comment on attachment 346069 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346069&action=review > Source/WebKit/ChangeLog:21 > + removed wincairo implementation plus some fixes/api changes Not "wincairo implementation" IIRC, but Windows implementation or AppleWin implementation. > Source/WebKit/ChangeLog:65 > + removed wincairo implementation plus some fixes/api changes Ditto. > Source/WebKit/Shared/PlatformPopupMenuData.cpp:40 > + , m_itemHeight(0) WebKit prefers member initializer. > Source/WebKit/Shared/PlatformPopupMenuData.h:58 > + int m_itemHeight; I think WebKit prefers member initializer. int m_clientPaddingLeft { 0 }; int m_clientPaddingRight { 0 }; int m_clientInsetLeft { 0 }; int m_clientInsetRight { 0 }; int m_popupWidth { 0 }; int m_itemHeight { 0 }; > Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp:36 > +#include <WebCore/BString.h> Is BString used? > Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp:47 > +using namespace std; Don't "using namespace std;" https://webkit.org/code-style-guidelines/#using-in-cpp > Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp:221 > + shouldAnimate = FALSE; You added this line. But, It seems for me that this line not needed. Does SystemParametersInfo change shouldAnimate if it fails? > Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.h:38 > +typedef struct HBITMAP__* HBITMAP; <windows.h> is included in WebCorePrefix.h. I think you can remove these typedefs. Comment on attachment 346069 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346069&action=review >> Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp:221 >> + shouldAnimate = FALSE; > > You added this line. But, It seems for me that this line not needed. Does SystemParametersInfo change shouldAnimate if it fails? This was added in the similar place in the legacy path after the non-legacy code was removed. "[Win] Resolve various static analyzer warnings in WebCore." https://github.sie.sony.com/WebCore/neko/commit/ca06693684d7a9bf69d7db2deb1c21ca823d6384#diff-d88e6bcdaacad433c28da98715f946d3 Sorry, public ref is: https://github.com/WebKit/webkit/commit/ca06693684d7a9bf69d7db2deb1c21ca823d6384#diff-d88e6bcdaacad433c28da98715f946d3 Created attachment 346198 [details]
Patch
Comment on attachment 346198 [details] Patch Clearing flags on attachment: 346198 Committed r234442: <https://trac.webkit.org/changeset/234442> All reviewed patches have been landed. Closing bug. |