Summary: | [Win] Implement Authentication dialog in MiniBrowser | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||||||||
Component: | Tools / Tests | Assignee: | Basuke Suzuki <Basuke.Suzuki> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, Basuke.Suzuki, bfulgham, buildbot, commit-queue, don.olmstead, lforschler, pvollan | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=189846 | ||||||||||||
Attachments: |
|
Description
Basuke Suzuki
2017-07-19 17:58:21 PDT
Created attachment 315997 [details]
Implemented
Attachment 315997 [details] did not pass style-queue:
ERROR: Tools/MiniBrowser/win/Common.cpp:671: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
ERROR: Tools/MiniBrowser/win/Common.cpp:681: One line control clauses should not use braces. [whitespace/braces] [4]
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:1: Should have a space between // and comment [whitespace/comments] [4]
ERROR: Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:34: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:97: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
ERROR: Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:107: One line control clauses should not use braces. [whitespace/braces] [4]
Total errors found: 7 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 316005 [details]
Fix style
(In reply to Build Bot from comment #2) > ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:0: No copyright > message found. You should have a line: "Copyright [year] <Copyright Owner>" > [legal/copyright] [5] > ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:1: Should have a > space between // and comment [whitespace/comments] [4] This file is the one VisualStudio generates. Created attachment 316007 [details]
Fix style
Attachment 316007 [details] did not pass style-queue:
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Tools/MiniBrowser/win/MiniBrowserLibResource.h:1: Should have a space between // and comment [whitespace/comments] [4]
Total errors found: 2 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 316007 [details] Fix style Attachment 316007 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4156792 New failing tests: imported/w3c/IndexedDB-private-browsing/idbfactory_open12.html Created attachment 316018 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
I have no idea why my change for Windows app break ios-sim layout test. Also I don't want to fix generated file because next modification causes that change useless. Comment on attachment 316007 [details] Fix style View in context: https://bugs.webkit.org/attachment.cgi?id=316007&action=review iOS test failure unrelated > Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:40 > +extern HRESULT DisplayAuthDialog(std::wstring& username, std::wstring& password); This should go in a header (In reply to Alex Christensen from comment #10) > Comment on attachment 316007 [details] > Fix style > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316007&action=review > > iOS test failure unrelated > > > Tools/MiniBrowser/win/ResourceLoadDelegate.cpp:40 > > +extern HRESULT DisplayAuthDialog(std::wstring& username, std::wstring& password); > > This should go in a header I agree with that, but basic architecture of this app is very new to me which is very confusing. The "Common.cpp" file has no header file and directly included from "WinMain.cpp" so that no chance to get the definition easily. Other references to those defined method seems to be called inside Common.cpp as the result of event. I am very new to Windows app programming and have no idea with this structure. Comment on attachment 316007 [details]
Fix style
MiniBrowser isn't exactly the most well-organized project anyways. Let's just land this.
Comment on attachment 316007 [details] Fix style Clearing flags on attachment: 316007 Committed r219830: <http://trac.webkit.org/changeset/219830> All reviewed patches have been landed. Closing bug. (In reply to Alex Christensen from comment #12) > Comment on attachment 316007 [details] > Fix style > > MiniBrowser isn't exactly the most well-organized project anyways. Let's > just land this. Thanks! |