| Summary: | Show prompt for device orientation access if the client does not implement the corresponding API delegate | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
| Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | achristensen, beidson, ggaren, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Chris Dumez
2019-04-16 10:51:54 PDT
Created attachment 367545 [details]
Patch
Comment on attachment 367545 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367545&action=review > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.h:31 > +#import <wtf/CompletionHandler.h> > +#import <wtf/text/WTFString.h> wtf/Forward.h should have everything necessary for this. > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.mm:35 > + String alertText = makeString('"', host, "\" Would Like to Access Motion and Orientation"); These strings need to be localized. > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.mm:40 > + auto completionBlock = makeBlockPtr([completionHandler = WTFMove(completionHandler)](bool shouldAllow) mutable { I think this code would look better if we made a BlockPtr twice. (In reply to Alex Christensen from comment #3) > Comment on attachment 367545 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367545&action=review > > > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.h:31 > > +#import <wtf/CompletionHandler.h> > > +#import <wtf/text/WTFString.h> > > wtf/Forward.h should have everything necessary for this. Ok. > > > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.mm:35 > > + String alertText = makeString('"', host, "\" Would Like to Access Motion and Orientation"); > > These strings need to be localized. Oops. > > > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.mm:40 > > + auto completionBlock = makeBlockPtr([completionHandler = WTFMove(completionHandler)](bool shouldAllow) mutable { > > I think this code would look better if we made a BlockPtr twice. How do I make a BlockPtr twice with a single CompletionHandler? Created attachment 367550 [details]
Patch
Comment on attachment 367550 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367550&action=review > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.mm:40 > + auto completionBlock = makeBlockPtr([completionHandler = WTFMove(completionHandler)](bool shouldAllow) mutable { Yep, we definitely need something like this. > Source/WebKit/UIProcess/Cocoa/WKOrientationAccessAlert.mm:44 > + UIAlertAction* cancelAction = [UIAlertAction actionWithTitle:@"Cancel" style:UIAlertActionStyleCancel handler:[completionBlock](UIAlertAction *action) { Cancel and Allow also should be localized. Created attachment 367551 [details]
Patch
Created attachment 367558 [details]
Patch
Alex, ping review? Does it look good to you now? Committed r244360: <https://trac.webkit.org/changeset/244360> |