Cleanup InternalSettings
Created attachment 413991 [details] Patch
Created attachment 413993 [details] Patch
Created attachment 413998 [details] Patch
Comment on attachment 413998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413998&action=review > LayoutTests/fast/scrolling/rtl-scrollbars-text-selection-scrolled.html:5 > - internals.settings.setUserInterfaceDirectionPolicy("View"); > + internals.settings.setUserInterfaceDirectionPolicy("System"); Huh, do these tests still pass if you change the preference on your machine, now?
(In reply to Tim Horton from comment #4) > Comment on attachment 413998 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=413998&action=review > > > LayoutTests/fast/scrolling/rtl-scrollbars-text-selection-scrolled.html:5 > > - internals.settings.setUserInterfaceDirectionPolicy("View"); > > + internals.settings.setUserInterfaceDirectionPolicy("System"); > > Huh, do these tests still pass if you change the preference on your machine, > now? All I changed was the spelling of the enum value. I don’t think anything else will have changed.
(In reply to Sam Weinig from comment #5) > (In reply to Tim Horton from comment #4) > > Comment on attachment 413998 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=413998&action=review > > > > > LayoutTests/fast/scrolling/rtl-scrollbars-text-selection-scrolled.html:5 > > > - internals.settings.setUserInterfaceDirectionPolicy("View"); > > > + internals.settings.setUserInterfaceDirectionPolicy("System"); > > > > Huh, do these tests still pass if you change the preference on your machine, > > now? > > All I changed was the spelling of the enum value. I don’t think anything > else will have changed. To be even clearer, here was the old code that mapped View to System: - if (equalLettersIgnoringASCIICase(policy, "content")) { - settings().setUserInterfaceDirectionPolicy(UserInterfaceDirectionPolicy::Content); - return { }; - } - if (equalLettersIgnoringASCIICase(policy, "view")) { - settings().setUserInterfaceDirectionPolicy(UserInterfaceDirectionPolicy::System); - return { }; - } - return Exception { InvalidAccessError }; Now it just maps "System" to System directly.
Committed r269784: <https://trac.webkit.org/changeset/269784> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413998 [details].
<rdar://problem/71374775>
(In reply to Sam Weinig from comment #6) > (In reply to Sam Weinig from comment #5) > > (In reply to Tim Horton from comment #4) > > > Comment on attachment 413998 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=413998&action=review > > > > > > > LayoutTests/fast/scrolling/rtl-scrollbars-text-selection-scrolled.html:5 > > > > - internals.settings.setUserInterfaceDirectionPolicy("View"); > > > > + internals.settings.setUserInterfaceDirectionPolicy("System"); > > > > > > Huh, do these tests still pass if you change the preference on your machine, > > > now? > > > > All I changed was the spelling of the enum value. I don’t think anything > > else will have changed. > > To be even clearer, here was the old code that mapped View to System: > > - if (equalLettersIgnoringASCIICase(policy, "content")) { > - > settings().setUserInterfaceDirectionPolicy(UserInterfaceDirectionPolicy:: > Content); > - return { }; > - } > - if (equalLettersIgnoringASCIICase(policy, "view")) { > - > settings().setUserInterfaceDirectionPolicy(UserInterfaceDirectionPolicy:: > System); > - return { }; > - } > - return Exception { InvalidAccessError }; > > Now it just maps "System" to System directly. Oh! Naively assumed "system" meant "app wide" and "view" meant "scoped to the view", since it is possible for those to differ. Now I wonder which it really is :D Anyway, making them all the same is A++.