Closed Bug 1235941 Opened 4 years ago Closed 4 years ago

Firefox doesn't react to GDK scale factor changes

Categories

(Core :: Widget: Gtk, defect)

46 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- affected
firefox47 --- fixed

People

(Reporter: marco, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Firefox doesn't react to DPI changes. If you open it when DPI hasn't been changed yet (as soon as the system starts), you end up with very small icons and text.
Other GTK applications instead (e.g. the GNOME terminal) correctly increase the size of their UI elements.
See Also: → 1228570
We should use monitors-changed signal.  Patch is coming.
Assignee: nobody → m_kato
Use monitors-changed signal to detect DPI change.  Also, we should update
mBounds for current scaling setting.

Review commit: https://reviewboard.mozilla.org/r/34725/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34725/
Attachment #8718739 - Flags: review?(karlt)
Comment on attachment 8718739 [details]
MozReview Request: Bug 1235941 - Detect DPI change for GTK3. r?karlt

https://reviewboard.mozilla.org/r/34725/#review31517

::: widget/gtk/nsWindow.cpp:3375
(Diff revision 1)
> +  // Update mBounds to current scaling setting
> +  Resize(mBounds.x, mBounds.y, mBounds.width, mBounds.height, true);

I doubt requesting a resize from the window manager is the right thing to do here.  GDK should be managing that for us.
What change in mBounds are you expecting here?
GetDesktopToDeviceScale() returns 1.0 and so Resize() won't change mBounds.

::: widget/gtk/nsWindow.cpp:3845
(Diff revision 1)
> +        g_signal_connect(gtk_widget_get_screen(mShell),
> +                         "monitors-changed",

"The ::monitors-changed signal is emitted when the number, size or position of
the monitors attached to the screen change."  That is the signal that
_gdk_x11_screen_set_window_scale emits, but it is also emitted in other
situations.  I think notify::scale-factor on GtkWidget is more appropriate.
configure-event on GtkWidget may be another option, but that's assuming a
little about the implementation.

GTK also has another control affecting scaling, changes in which are signalled
through both notify::resolution on GdkScreen and notify::gtk-xft-dpi on
GtkSettings.  Either of those should be fine I expect, but it's also fine to
leave this for now if you like.
Attachment #8718739 - Flags: review?(karlt)
https://reviewboard.mozilla.org/r/34725/#review31517

> I doubt requesting a resize from the window manager is the right thing to do here.  GDK should be managing that for us.
> What change in mBounds are you expecting here?
> GetDesktopToDeviceScale() returns 1.0 and so Resize() won't change mBounds.

When dpi is changed, although configure_event is fired, size-allocate isn't fired.  I should call OnSizeAllocate using GtkAllocation after calling  gtk_widget_get_allocation.

> "The ::monitors-changed signal is emitted when the number, size or position of
> the monitors attached to the screen change."  That is the signal that
> _gdk_x11_screen_set_window_scale emits, but it is also emitted in other
> situations.  I think notify::scale-factor on GtkWidget is more appropriate.
> configure-event on GtkWidget may be another option, but that's assuming a
> little about the implementation.
> 
> GTK also has another control affecting scaling, changes in which are signalled
> through both notify::resolution on GdkScreen and notify::gtk-xft-dpi on
> GtkSettings.  Either of those should be fine I expect, but it's also fine to
> leave this for now if you like.

GtkWebKit seems to use "notify::scale-factor".  So I think that "notify::scale-factor" is better instead.
Attachment #8718739 - Flags: review?(karlt)
Comment on attachment 8718739 [details]
MozReview Request: Bug 1235941 - Detect DPI change for GTK3. r?karlt

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34725/diff/1-2/
Comment on attachment 8718739 [details]
MozReview Request: Bug 1235941 - Detect DPI change for GTK3. r?karlt

https://reviewboard.mozilla.org/r/34725/#review32431

::: widget/gtk/nsWindow.cpp:6005
(Diff revision 2)
> +scale_changed_cb (GtkWidget *widget)

No space before '(' and position change for '*' "GtkWidget* " is Gecko style for new code.

Please specify the GParamSpec* and gpointer parameters explicitly, instead of depending on platform calling convention to drop the trailing unused parameters.  I don't mind whether you include the pspec and user_data names or not.

::: widget/gtk/nsWindow.cpp:6013
(Diff revision 2)
> +    // configure_event is already fired before scaling_facotr signal,

scaling_facotr -> scale-factor

Thanks for the analysis.
Attachment #8718739 - Flags: review?(karlt) → review+
Marco, this may not resolve your issue.  If not please feel free to dupe for adding a handler for notify::resolution on GdkScreen or notify::gtk-xft-dpi on GtkSettings.
Summary: Firefox doesn't react to DPI changes → Firefox doesn't react to GDK scale factor changes
https://hg.mozilla.org/mozilla-central/rev/38f6325a50c3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Blocks: 1251432
You need to log in before you can comment on or make changes to this bug.