RFE: Always use my font choices preference

[Kevin Fenzi]

It would be nice if midori had a preference to 'always use my font choices' instead of allowing the remote site to pick what fonts to use. In some cases people really prefer whatever font they have chosen and don't want to use whatever the remote site picks.

[Cris Dywan (kalikiana)]

There's no need to keep seprarate labels in WebKitWebSettings - if you saw it elsewhere it was because WebKit built-in settings were used and didn't have nice, translated labels.
I wouldn't bother to add authors for one-liners like in katze-utils.c. Your credit still appears in the git log and the headers would be crowded very easily otherwise ;-)
And "Since: 0.4.2 now effectively".
^^ Updated patch attached.

midori_browser_settings_notify leaks "family".

_midori_browser_forced_font sort of touches a general question: what to use user-stylesheet-uri for. There's been ideas but somehow we were stuck not using it at all. I'll add some names on the bug for that reason.
I think it would be better to do this from inside MidoriWebSettings to keep it flexible.

Do we actually need to duplicate the font family and size values? Kevin? Maybe we should just operate on both proportional and monospace font and override with whatever given values?

[Peter Hatina (phatina)]

Christian, I'm okay with credits and shot-liners (just habit from work). To user-stylesheet-uri, can anyone tell me, why not to use this? It look like a good way, how to pass to webkit our desired css - and it does not "blink" like user-style sheets addon does. which loads a webpage and when rendered, it applies the style sheet, which causes to redraw the whole page. To font size, maybe you are right, the size could be ommited.

[gue5t gue5t (gue5t)]

It's a bad idea to use user-stylesheet-uri in this manner because the user may have already set it in their preferences, or other code (e.g. extensions) may want to use it as well. I think if midori is going to programmatically assign user-stylesheet-uri to apply global theming, it should do so in such a way that each component's changes to the property are reversible and act as additions to the old value rather than simple assignments.

This could be achieved via a wrapper with add and remove functions for style rulesets and which composes a user-stylesheet-uri from the known active rulesets and the value of the setting from config, but any solution should ensure that midori does not destroy user-specified values for the setting--it's the most effective way to unconditionally apply styling, as userstyles are known to "flash" and require javascript.

[Peter Hatina (phatina)]

So if we want to have the gui-settings for a forced font in web pages, would you prefer some other way? Let's say, programmatically turn on user styles/scripts addon (if necessary) and maybe create some *.css in ~/.local/share/midori/styles? On #midori (freenode), I got that, we can set user styles for certain web pages.

[gue5t gue5t (gue5t)]

If we want to have a GUI setting for forcing the font, I'm not against using the user-stylesheet-uri to do so, as it would definitely be the most effective solution here--no flashing, no extensions, and no javascript required. That said, I do think that user-stylesheet-uri should not be used directly, exactly because it's the most effective way to set global styles, and multiple components (including the end user) may want access to that ability. A midori component which multiplexed the property by keeping track of registered portions of the property and summed them into an actual value to use seems like it would work to make user-stylesheet-uri a reusable method of setting global style properties.

API could look something like

guint midori_settings_add_style_rule(MidoriWebSettings *settings, gchar *rule); //returns rule id, which is used to remove the rule later
midori_settings_remove_style_rule(MidoriWebSettings *settings, guint id); //removes the rule set with the given id

each of which would update the set of style rules and recalculate a resultant value for user-stylesheet-uri.

_midori_browser_forced_font could then be implemented as follows:

static void
_midori_browser_forced_font (MidoriBrowser* browser, const gchar* family, gint size, gboolean enabled)
{
    MidoriWebSettings* midori_settings = browser->settings;
    static guint rule_id = 0;

    if (enabled)
    {
        GString* css_str = g_string_new ("* { font: ");
        g_string_append_printf (css_str, "%dpt ", size);
        g_string_append_printf (css_str, "%s !important; }", family);

        rule_id = midori_settings_add_style_rule (css_str->str);

        g_string_free (css_str, TRUE);
    }
    else if (rule_id)
    {
     midori_settings_remove_style_rule (settings, rule_id);
    }
}

[Peter Hatina (phatina)]

Yes, that sounds reasonable. I am okay with this, what about you, Christian?

[Cris Dywan (kalikiana)]

I like the proposed midori_settings_add/remove_style_rule. Maybe also midori_settings_replace?

I'm thinking, and that's what I brought up in IRC, passing a string identifier such as "adblock" or "addons" might be nice, so that you don't have to keep around the ID. And you could still construct an ID with an incremental counter. Some ideas here:

midori_settings_add_style_rule (settings, gchar* rule_id, gchar* style) ← could double as 'replace'
midori_settings_remove_style_rule (settings, gchar* rule_id)
midori_settings_remove_all_style_rules (settings, gchar* rule_id) matches "adblock-1" and "adblock-2" if you pass "adblock"

But I'm not decided at all. All the APIs I can come up with feel slightly icky in one way or another.

[Alexander Butenko (avb)]

i like the idea. That should simplify code a bit.
Also i believe will be nice to have something like midori_settings_replace_style_rule () which updates rule in the fastest way as possible.

But before we need to make sure that this function will inject style during domready or earlier javascript state so first displayed result will appear with style changes already applied.

Also need to check if this style will not be applied more then 1 time to a single page as this can cause performance issues.

[gue5t gue5t (gue5t)]

This patch adds the API to MidoriWebSettings and shows its usage in the addons extensions for userstyles without include/exclude lists.

[Peter Hatina (phatina)]

I added the proposed api, and dropped forced font size, which I consider useless (after time :)

[Cris Dywan (kalikiana)]

So it was not easy to decide between two nice patches, both implementing the API a bit differently. Eventually I set my mind on simplicity, so I went for string-based ID's, enforcing font sizes with only a boolean but the existing font family, handling it inside web settings, not browser. Further more I kept the addon change a bit simpler. And I made a separate change for allowing special pages.

[gue5t gue5t (gue5t)]

At the moment there are a few outstanding bugs, which kalikiana and I were looking into on IRC, but for the record:
- changing font doesn't update the font-forcing css until that option is toggled (fix pending)
- user addons is not applying its stylesheet when enabled (a mere oversight)
- "alway use my font choices" is not applying its css on startup--it must be toggled first (another simple oversight)
- when multiple hash table entries are concatenated it seems that there's some corruption of the data--maybe there are some strings being used as-passed which are later freed?
- existing values of user-stylesheet-uri (such as from the previous run) will affect pages until a style rule is added, at which point it will be replaced outright; this can have odd results if styles are not added.
- there is no alternative to user-stylesheet-uri for end users (though this is not critical really; user addons mostly suffices now, though kiosk-style applications and users who want a static style would benefit from a direct replacement in MidoriWebSettings)

I don't mean to be rude, but I feel that those of these issues that pertain directly to the implementation of the style api were solved elegantly and more efficiently (c.f. code paths for adding or replacing a style) in the patch I composed. Maybe it's worth looking over more closely (in particular, the list-based caching of style string lengths and the midori-user-stylesheet-uri property)? An adaptation of that implementation to use the string-id API would be quite straightforward--perhaps maintain a list of the string ids in parallel with the numerical id list, or a hash table of string->numerical id.

Alternatively we can approach the bugs here directly, but I'm not very satisfied with the efficiency of regenerating the entire global style via repeated appends on every update.

[Peter Hatina (phatina)]

Please, can you show me your css, which causes mentioned issues?

[Cris Dywan (kalikiana)]

So I need to add here, changing the approach, or adding the user-stylesheet-uri support can still be done. In part I wanted to accelerate development by having existing API that extensions can already use. And I'd like adblock to do so as well. This way we can actually measure performance for instance.
- I fixed the issue with changing the font family, a notify was missing.
- Addons is applying the stylesheet now, the code was for some reason doing the opposite.

[gue5t gue5t (gue5t)]

The corruption seems to be caused by midori_web_settings_add_style using strings after they're freed--its arguments are freed by the caller in the usage cases implemented so far. The signature should definitely be a const gchar* for the rule_id and should either be transfer-all or another const gchar* for the style itself. If it's transfer-all, the string will have to be freed in remove_style.

[Cris Dywan (kalikiana)]

Good catch, you're right. I completely overlooked that we need to copy the styles. Done.

[Cris Dywan (kalikiana)]

I added support for user-stylesheet-uri again, also working in private browsing.

Now what's missing is replacing the stylesheet generation with a list-based approach and seeing if it's faster.

[Cris Dywan (kalikiana)]

See bug 883879 for a follow-up bug.