Bug 186697

Created attachment 342849 [details]
patch

Comment on attachment 342849 [details]
patch

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

> Source/WebCore/editing/mac/FrameSelectionMac.mm:42
> +    CGFloat screenHeight = screenRectForPrimaryScreen().height();
> +    if (screenHeight > 0)
>          bounds.origin.y = (screenHeight - (bounds.origin.y + bounds.size.height));
> -    } else
> +    else
>          bounds = CGRectZero;    

Might be even better to use the toUserSpaceForPrimaryScreen function instead of calling screenRectForPrimaryScreen directly.

(In reply to Darin Adler from comment #2)
> Comment on attachment 342849 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=342849&action=review
> 
> > Source/WebCore/editing/mac/FrameSelectionMac.mm:42
> > +    CGFloat screenHeight = screenRectForPrimaryScreen().height();
> > +    if (screenHeight > 0)
> >          bounds.origin.y = (screenHeight - (bounds.origin.y + bounds.size.height));
> > -    } else
> > +    else
> >          bounds = CGRectZero;    
> 
> Might be even better to use the toUserSpaceForPrimaryScreen function instead
> of calling screenRectForPrimaryScreen directly.

Great thanks! I'll update that

Committed r232935: <https://trac.webkit.org/changeset/232935>

Looks like we have build failures on macOS 32-bit after this patch:

https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20%2832-bit%20Build%29/builds/6741/steps/compile-webkit/logs/stdio


......
/Volumes/Data/slave/highsierra-32bit-release/build/Source/WebCore/platform/PlatformScreen.h:93:11: note: candidate function not viable: no known conversion from 'CGRect' to 'const NSRect' (aka 'const _NSRect') for 1st argument
FloatRect toUserSpaceForPrimaryScreen(const NSRect&);
          ^
2 errors generated.

** BUILD FAILED **
The following build commands failed:
	CompileC /Volumes/Data/slave/highsierra-32bit-release/build/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/UnifiedSource12-mm.o /Volumes/Data/slave/highsierra-32bit-release/build/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource12-mm.mm normal i386 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)


https://build.webkit.org/builders/Apple%20Sierra%20Release%20%2832-bit%20Build%29/builds/11856/steps/compile-webkit/logs/stdio
....
/Volumes/Data/slave/sierra-32bit-release/build/Source/WebCore/platform/PlatformScreen.h:93:11: note: candidate function not viable: no known conversion from 'CGRect' to 'const NSRect' (aka 'const _NSRect') for 1st argument
FloatRect toUserSpaceForPrimaryScreen(const NSRect&);
          ^
2 errors generated.

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/slave/sierra-32bit-release/build/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/UnifiedSource12-mm.o /Volumes/Data/slave/sierra-32bit-release/build/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource12-mm.mm normal i386 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

It worked for me locally. I will revert this and take a look.

Re-opened since this is blocked by bug 186783

Created attachment 342961 [details]
patch

This should fix the 32-bit build issue.

(In reply to Nan Wang from comment #8)
> Created attachment 342961 [details]
> patch
> 
> This should fix the 32-bit build issue.

great thanks!

Comment on attachment 342961 [details]
patch

Rejecting attachment 342961 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 342961, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/8236008

Committed r232944: <https://trac.webkit.org/changeset/232944>