Bug 138943 - Preview popover needs minimum and maximum sizes
Summary: Preview popover needs minimum and maximum sizes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-20 16:47 PST by Beth Dakin
Modified: 2014-11-20 18:10 PST (History)
4 users (show)

See Also:


Attachments
Patch (9.72 KB, patch)
2014-11-20 17:45 PST, Beth Dakin
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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!