After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 673108 - Font settings and monospace fonts don't work
Font settings and monospace fonts don't work
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: general
3.6.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
evolution[webkit]
Depends on:
Blocks:
 
 
Reported: 2012-03-29 19:48 UTC by Milan Crha
Modified: 2013-09-13 01:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screen shot (94.88 KB, image/png)
2012-03-29 20:00 UTC, Milan Crha
  Details
Proposed patch (16.65 KB, patch)
2012-04-03 10:09 UTC, Dan Vrátil
needs-work Details | Review
Fixed patch (11.53 KB, patch)
2012-04-10 15:06 UTC, Dan Vrátil
committed Details | Review
Fix (705 bytes, patch)
2012-04-13 10:58 UTC, Dan Vrátil
committed Details | Review

Description Milan Crha 2012-03-29 19:48:02 UTC
There are some issues with fonts in webkit, like default monospace font is not monospace and other issues (see the attached image for an example, on the left if 3.4.0, on the right 3.5.1 with webkit - check colors and font being used).

After a discussion on IRC, Matthew made it simpler (cite down to "Dan..."):
I'd split the problem up:
1) the CSS code should use the font names according to EWebView.  period.
2) EWebView needs to initialize its font names according to user preferences

We can initialize the font properties from here:
http://git.gnome.org/browse/evolution/tree/modules/mail/e-mail-config-web-view.c
but maybe only apply the custom font preference if the instance is an EMailDisplay rather than all EWebViews (EMailDisplay being a subclass of EWebView...)

Dan also produced preliminar patch, which I think will need changes due to Matthew's suggestion. Better to check it here, before committing.
Comment 1 Milan Crha 2012-03-29 20:00:28 UTC
Created attachment 210898 [details]
screen shot

This shows 3.4.0, 3.5.1 and 3.5.1 patched. The patched one looks better, but still far from the nice view of 3.4.0.

I would like to suggest to merge link colors as well, people are use to it (or at least me).
Comment 2 Milan Crha 2012-04-02 12:16:34 UTC
This [1] shows a special webkit CSS property for font antialiasing (bug is about it not working for GDI), still, not forcing hinting and antialiasing changes in webkit makes the font used as expected (of course, they do not get style changes, thus no fun further). I filled this [2] in their bugzilla.

Here lefts the part about non-monospace fonts being used.

[1] https://bugs.webkit.org/show_bug.cgi?id=54004
[2] https://bugs.webkit.org/show_bug.cgi?id=82889
Comment 3 Dan Vrátil 2012-04-03 10:09:50 UTC
Created attachment 211211 [details] [review]
Proposed patch

This is the proposed patch.

> We can initialize the font properties from here: 
> /modules/mail/e-mail-config-web-view.c but maybe only apply the custom font  
> preference ...

We can't. In webkit you can only set font family and font size, but not font style (and I'm not going to patch WebKit for this, Milan :) ), so the only way to do this properly is through EMFormatHTML, which will write the font preferences to a global CSS style of the rendered email (does not include text/html emails, though not to override their own font definitions (if any)).

If you were "serif 10" and "monospace 10" came from, those are default values for monospace and normal font in WebKitWebSettings.
Comment 4 Milan Crha 2012-04-03 16:54:48 UTC
I didn't test it yet, but why to force antialiased smoothing? My point was to use system preferences, aka follow user's choice (different device (monitor) can like different setup). The value is also stored in GSettings, as org.gnome.settings-daemon.plugins.xsettings, key "hinting" and "antialiasing", though an easier would be to use GtkSettings, property "gtk-xft-rgba" in combination with those other "gtk-xft-*". I just realized they also provide "gtk-font-name", but not the one for monospace.
Comment 5 Milan Crha 2012-04-04 18:07:26 UTC
I also tested the patch now, and it doesn't work fully:
a) compiler warning
   e-mail-request.c: In function 'mail_request_send_async':
   e-mail-request.c:642:10: warning: unused variable 'decoded_uri'
b) crash which I reported as bug #673525 (maybe unrelated)
c) changing Font preferences requires restart of evolution
d) (from comment #4) do not hardcode -webkit-font-smoothing: antialiased;
   in the webview.css, use current system setting instead
Comment 6 Milan Crha 2012-04-04 18:15:41 UTC
(In reply to comment #5)
> a) compiler warning
>    e-mail-request.c: In function 'mail_request_send_async':
>    e-mail-request.c:642:10: warning: unused variable 'decoded_uri'

Oops, your patch does not touch this part of the code, I'm sorry, I thought it does.
Comment 7 Matthew Barnes 2012-04-04 18:17:56 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > a) compiler warning
> >    e-mail-request.c: In function 'mail_request_send_async':
> >    e-mail-request.c:642:10: warning: unused variable 'decoded_uri'
> 
> Oops, your patch does not touch this part of the code, I'm sorry, I thought it
> does.

That's a leftover from bug #673430.  Just fix it.
Comment 8 Milan Crha 2012-04-05 09:28:50 UTC
(In reply to comment #7)
> That's a leftover from bug #673430.  Just fix it.

OK, committed.
Comment 9 Dan Vrátil 2012-04-10 15:06:01 UTC
Created attachment 211747 [details] [review]
Fixed patch

Reworked patch

EWebView:
EWebView now has a virtual method set_fonts() through which subclasses can assign their preferred font settings. If any of the fonts returned via set_fonts() is NULL or subclass does not reimplement the method at all, EWebView falls back to system font settings (+ aliasing). This "fallback" is used by contacts or tasks previews, which don't have any special font settings.

EMailDisplay:
EMailDisplay reimplements set_fonts() in such way, that if org.gnome.evolution.mail.use_custom_font is disabled then it returns NULL (and thus EWebView falls back to system fonts), otherwise it returns user-defined fonts for mail view.

Both EWebView and EMailDisplay listen to changes in respective GSettings properties, so when system font is changed, the font is changed immediatelly in EWebView as well. The same goes for EMailDisplay.

The fonts and aliasing are passed to WebKit via so called user stylesheet, which is a global CSS stylesheet applied to every single page and frame.
Comment 10 Milan Crha 2012-04-11 08:23:44 UTC
The function definition should have 'void' in parameters, which is missing here:
   e_web_view_get_default_settings ()
Nice the above prototype didn't match its public definition (from the .h file, where is the 'void' missing too).

There are couple places with missing space between function name and parameters, like here:
   e_web_view_update_fonts(E_WEB_VIEW (display));

Apart of the above nitpicks it looks good, I do not see the e_web_view_update_fonts() being called when I change antialiasing in Advances Settings, though the key changed, but even if I make it rebuild CSS, like when i change "Use system font" in evolution, then the antialiasing isn't picked properly by webkit. Nothing you can do with it, I'm afraid, thus I've no problem with "no change in view with different antialiasing value". Only fix the above two things, maybe check if you can see why the change in the antialiasing key is not notified by evolution, and commit. Thanks.
Comment 11 Dan Vrátil 2012-04-12 10:55:01 UTC
Fixed formatting and automatic update of antialiasing when gsettings key is changed. The fact that WebKit does not seem to respect it is an another issue that I really can do nothing about.

http://git.gnome.org/browse/evolution/commit/?id=13762515153f9e254e5c17fb747ad76121f8710c
Comment 12 Milan Crha 2012-04-13 08:39:24 UTC
One regression:
a) Edit->Preferences->Mail Preferences
   [ ] Use the same fonts as other application
   Standard font    [ any proportional font | 11 ]
   Fixed width font [    Courier 10 Pith    | 11 ]
b) open a bugzilla email, which is surely text/plain
c) observe the headers are using proper Standard font, but the text/plain
   part is not using the Courier font, it uses some other, proportional font

If I check:
   [x] Use the same fonts as other application
then the text/plain part of the same message shows fixed width font as expected.
Comment 13 Dan Vrátil 2012-04-13 10:07:03 UTC
Probably a WebKit bug. The problem lies in name of the font, which is "Courier 10 Pith". 

The value is correctly assigned to font-family in the global CSS style, but WebKit is probably not able to interpret it (it expects "Courier" or something like that, not "Courier 10 Pith"). Because the standard font is applied to everything (<body>) and the monospace font is applied only to a subset of elements (<pre>,<code>,class="pre"), WebKit falls back to the standard font in the <pre> parts as it is unable to correctly process "Courier 10 Pith" name.

Proposed solution:
Check the font name and if it contains a number then take only the first part of the font name before the number and throw away the rest.
Comment 14 Dan Vrátil 2012-04-13 10:58:01 UTC
Created attachment 211979 [details] [review]
Fix

Apologies to WebKit, it is actually my bug. Patch is attached.
Comment 15 Milan Crha 2012-04-16 11:58:13 UTC
Works for me, please commit
Comment 16 Dan Vrátil 2012-04-16 12:20:09 UTC
Committed to master as 6762b1238d6c2e38aeca76ec266b7e6b82a36f19

http://git.gnome.org/browse/evolution/commit/?id=6762b1238d6c2e38aeca76ec266b7e6b82a36f19