RESOLVED FIXED 183231
Scrollbar preferences are ignored when the WebContent process doesn't have access to the WindowServer.
https://bugs.webkit.org/show_bug.cgi?id=183231
Summary Scrollbar preferences are ignored when the WebContent process doesn't have ac...
Per Arne Vollan
Reported 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.
Attachments
Patch (16.55 KB, patch)
2018-02-28 16:32 PST, Per Arne Vollan
no flags
Patch (18.80 KB, patch)
2018-03-01 10:12 PST, Per Arne Vollan
bfulgham: review+
Patch (18.57 KB, patch)
2018-03-01 10:44 PST, Per Arne Vollan
no flags
Patch (18.03 KB, patch)
2018-03-01 11:32 PST, Per Arne Vollan
commit-queue: commit-queue-
Per Arne Vollan
Comment 1 2018-02-28 16:19:36 PST
Per Arne Vollan
Comment 2 2018-02-28 16:32:46 PST
Brent Fulgham
Comment 3 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! :-)
Brent Fulgham
Comment 4 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.
Brent Fulgham
Comment 5 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.
Per Arne Vollan
Comment 6 2018-03-01 10:12:22 PST
Per Arne Vollan
Comment 7 2018-03-01 10:44:10 PST
Per Arne Vollan
Comment 8 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 :)
Brent Fulgham
Comment 9 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.
Per Arne Vollan
Comment 10 2018-03-01 11:32:33 PST
Per Arne Vollan
Comment 11 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!
WebKit Commit Bot
Comment 12 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
Per Arne Vollan
Comment 13 2018-03-01 13:18:16 PST
Note You need to log in before you can comment on or make changes to this bug.