Bug 183231 - Scrollbar preferences are ignored when the WebContent process doesn't have access to the WindowServer.
Summary: Scrollbar preferences are ignored when the WebContent process doesn't have ac...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-28 16:18 PST by Per Arne Vollan
Modified: 2018-03-01 13:18 PST (History)
4 users (show)

See Also:


Attachments
Patch (16.55 KB, patch)
2018-02-28 16:32 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (18.80 KB, patch)
2018-03-01 10:12 PST, Per Arne Vollan
bfulgham: review+
Details | Formatted Diff | Diff
Patch (18.57 KB, patch)
2018-03-01 10:44 PST, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (18.03 KB, patch)
2018-03-01 11:32 PST, Per Arne Vollan
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2018-02-28 16:18:47 PST
When the WebContent process doesn't have access to the WindowServer, the scrollbars are always of the overlay type.
Comment 1 Per Arne Vollan 2018-02-28 16:19:36 PST
<rdar://problem/37793457>
Comment 2 Per Arne Vollan 2018-02-28 16:32:46 PST
Created attachment 334784 [details]
Patch
Comment 3 Brent Fulgham 2018-02-28 17:12:21 PST
Comment on attachment 334784 [details]
Patch

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

> Source/WebKit/WebProcess/WebProcess.cpp:222
> +

You can revert this change and make the patch smaller! :-)
Comment 4 Brent Fulgham 2018-02-28 17:15:49 PST
Build errors:

In file included from ./platform/mac/NSScrollerImpDetails.mm:29:
./platform/mac/NSScrollerImpDetails.h:39:17: error: no type named 'optional' in namespace 'std'
    static std::optional<bool> m_useOverlayScrollbars;
           ~~~~~^
./platform/mac/NSScrollerImpDetails.h:39:25: error: expected member name or ';' after declaration specifiers
    static std::optional<bool> m_useOverlayScrollbars;
    ~~~~~~~~~~~~~~~~~~~~^
In file included from /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource46-mm.mm:1:
./platform/mac/NSScrollerImpDetails.mm:37:36: error: no member named 'm_useOverlayScrollbars' in 'WebCore::ScrollerStyle'
std::optional<bool> ScrollerStyle::m_useOverlayScrollbars;
                    ~~~~~~~~~~~~~~~^
3 errors generated.
Comment 5 Brent Fulgham 2018-02-28 17:16:44 PST
Comment on attachment 334784 [details]
Patch

I think this seems good overall, but fails to build on any of the mac platforms. This might be an SDK difference between Sierra and High Sierra. Likely just a missing include. r- to fix the build.
Comment 6 Per Arne Vollan 2018-03-01 10:12:22 PST
Created attachment 334821 [details]
Patch
Comment 7 Per Arne Vollan 2018-03-01 10:44:10 PST
Created attachment 334826 [details]
Patch
Comment 8 Per Arne Vollan 2018-03-01 10:48:57 PST
(In reply to Brent Fulgham from comment #5)
> Comment on attachment 334784 [details]
> Patch
> 
> I think this seems good overall, but fails to build on any of the mac
> platforms. This might be an SDK difference between Sierra and High Sierra.
> Likely just a missing include. r- to fix the build.

Thanks for reviewing! It seems to build on the bots now :)
Comment 9 Brent Fulgham 2018-03-01 11:13:24 PST
Comment on attachment 334821 [details]
Patch

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

r=me

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:2518
> +		93500F3213FDE3BE0099EC24 /* NSScrollerImpDetails.h in Headers */ = {isa = PBXBuildFile; fileRef = 93500F3113FDE3BE0099EC24 /* NSScrollerImpDetails.h */; settings = {ATTRIBUTES = (Private, ); }; };

Did you change this on purpose? Fine if you did, but sometimes Xcode shuffles things around for arbitrary reasons (and you could revert this to make the patch smaller and easier to apply)

> Source/WebKit/WebProcess/WebProcess.cpp:222
> +

Please revert this whitespace-only change.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:65
> +#endif

We usually have these conditional includes in a separate area.
Comment 10 Per Arne Vollan 2018-03-01 11:32:33 PST
Created attachment 334829 [details]
Patch
Comment 11 Per Arne Vollan 2018-03-01 11:34:11 PST
(In reply to Brent Fulgham from comment #9)
> Comment on attachment 334821 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334821&action=review
> 
> r=me
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:2518
> > +		93500F3213FDE3BE0099EC24 /* NSScrollerImpDetails.h in Headers */ = {isa = PBXBuildFile; fileRef = 93500F3113FDE3BE0099EC24 /* NSScrollerImpDetails.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 
> Did you change this on purpose? Fine if you did, but sometimes Xcode
> shuffles things around for arbitrary reasons (and you could revert this to
> make the patch smaller and easier to apply)
> 
> > Source/WebKit/WebProcess/WebProcess.cpp:222
> > +
> 
> Please revert this whitespace-only change.
> 
> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:65
> > +#endif
> 
> We usually have these conditional includes in a separate area.

I have uploaded a modified patch for landing. Thanks for reviewing!
Comment 12 WebKit Commit Bot 2018-03-01 12:55:35 PST
Comment on attachment 334829 [details]
Patch

Rejecting attachment 334829 [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-01', 'validate-changelog', '--check-oops', '--non-interactive', 334829, '--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/6724526
Comment 13 Per Arne Vollan 2018-03-01 13:18:16 PST
Committed r229140: <https://trac.webkit.org/changeset/229140/webkit>.