[Webkit-unassigned] [Bug 142673] [GTK] Add settings to set font size in points, and deprecate the pixel size settings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=142673

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #325462|commit-queue?               |commit-queue-
              Flags|                            |

--- Comment #28 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 325462
  --> https://bugs.webkit.org/attachment.cgi?id=325462
Patch

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

OK, good, this passes my review. Only a couple minor comments remaining.

Now, because this patch adds new public API, we have a rule that it has to be approved by two GTK reviewers. I'm sure Carlos Garcia will want to be the second reviewer, so please ping him tomorrow and ask.

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:47
> +    // Set a DPI value of 96 for the test. This must be set before creating
> +    // the WebKitSettings object so that the notify::gtk-xft-dpi signal handler
> +    // of WebKitSettings won't get called to modify the initial font size values.
> +    GtkSettings* gtkSettings = gtk_settings_get_default();
> +    if (gtkSettings)
> +        g_object_set(gtkSettings, "gtk-xft-dpi", 96 * 1024, nullptr);

You need #if PLATFORM(GTK) guards around this, to avoid breaking WPE.

Also: I would add a second call to this down at the end of the test, and then verify that the font size in pixels changes when you change the DPI, but the font size in points does not.

Lastly, since the font size tests are now more complicated than just setting a value and verifying that it's changed, I would add a new test case function testFontSettings and split all of this out of testWebKitSettings.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171031/0226ff5c/attachment.html>