Bug 138943

Summary: Preview popover needs minimum and maximum sizes
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, conrad_shultz, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch thorton: review+

Description Beth Dakin 2014-11-20 16:47:16 PST
Preview popover needs minimum and maximum sizes

rdar://problem/18904651
Comment 1 Beth Dakin 2014-11-20 17:45:35 PST
Created attachment 242009 [details]
Patch
Comment 2 WebKit Commit Bot 2014-11-20 17:47:01 PST
Attachment 242009 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:398:  Missing space after ,  [whitespace/comma] [3]
ERROR: Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:405:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Tim Horton 2014-11-20 17:51:05 PST
Comment on attachment 242009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=242009&action=review

> Source/WebKit2/ChangeLog:12
> +        _hitTestResult, so we should computer a new origin rect based on the original 

computer!!

> Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:398
> +    if (targetSizeFitsInAvailableSpace(targetSize, maxSpaceAvailableOnYEdge) && targetSizeFitsInAvailableSpace(targetSize,largestPopoverSize))

you're missing some spaces after your commas
Comment 4 Beth Dakin 2014-11-20 17:53:04 PST
Thank you! http://trac.webkit.org/changeset/176429
Comment 5 Conrad Shultz 2014-11-20 17:59:50 PST
Comment on attachment 242009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=242009&action=review

> Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:363
> +    NSSize screenSize = [[NSScreen mainScreen] frame].size;

I'm not sure this will do what you want in all multi-screen cases. Is it guaranteed that keyboard focus is on the same screen as the presenting WKView?
Comment 6 Beth Dakin 2014-11-20 18:10:58 PST
http://trac.webkit.org/changeset/176430 Good call, Conrad!