| Summary: | [PlayStation] Enable TestWebKit | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Don Olmstead <don.olmstead> | ||||||
| Component: | Tools / Tests | Assignee: | Don Olmstead <don.olmstead> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | annulen, commit-queue, ews-watchlist, gyuyoung.kim, ross.kirsling, ryuan.choi, sergio, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Don Olmstead
2020-02-04 10:44:10 PST
Created attachment 389684 [details]
Patch
Comment on attachment 389684 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389684&action=review > Tools/TestWebKitAPI/playstation/PlatformUtilitiesPlayStation.cpp:54 > + // FIXME nit: probably best to leave off the FIXME if we're not gonna link a bug? Up to you for this case though. > Tools/TestWebKitAPI/playstation/PlatformWebViewPlayStation.cpp:31 > +#include <WebKit/WKPageConfigurationRef.h> > +#include <WebKit/WKRetainPtr.h> > +#include <WebKit/WebKit2_C.h> So long as these are stubs, we shouldn't need these includes or any of the param names below. > Tools/TestWebKitAPI/playstation/PlatformWebViewPlayStation.cpp:33 > +namespace TestWebKitAPI { Hmm, looking at the header, I'm surprised that we don't need to implement focus() or simulateAltKeyPress()...but if it works, it works. Comment on attachment 389684 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389684&action=review >> Tools/TestWebKitAPI/playstation/PlatformUtilitiesPlayStation.cpp:54 >> + // FIXME > > nit: probably best to leave off the FIXME if we're not gonna link a bug? Up to you for this case though. Since there is a return value I think adding some sort of FIXME is prudent so it doesn't get overlooked as the implementation lands. There's precedence in other port implementations. >> Tools/TestWebKitAPI/playstation/PlatformWebViewPlayStation.cpp:31 >> +#include <WebKit/WebKit2_C.h> > > So long as these are stubs, we shouldn't need these includes or any of the param names below. Removing >> Tools/TestWebKitAPI/playstation/PlatformWebViewPlayStation.cpp:33 >> +namespace TestWebKitAPI { > > Hmm, looking at the header, I'm surprised that we don't need to implement focus() or simulateAltKeyPress()...but if it works, it works. Looks like mac doesnt implement simulateAltKeyPress and only mac implements focus. I'll just add them for consistency. Created attachment 389711 [details]
Patch
Make Ross happy
Comment on attachment 389711 [details] Patch Clearing flags on attachment: 389711 Committed r255708: <https://trac.webkit.org/changeset/255708> All reviewed patches have been landed. Closing bug. |