Opened 9 years ago

Closed 6 years ago

#6118 closed enhancement (fixed)

[Patch] WebPositive Fallback fonts enhancement

Reported by: mt Owned by: leavengood
Priority: high Milestone: R1
Component: Applications/WebPositive Version: R1/alpha2
Keywords: Cc: linlongzhou@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

In findMatchingFontFamily() at WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp, Fallback fonts are constant.

        if (familyName.contains("Sans", false) != B_ERROR)
            strncpy(*fontFamily, "DejaVu Sans", B_FONT_FAMILY_LENGTH + 1);
        else if (familyName.contains("Serif", false) != B_ERROR)
            strncpy(*fontFamily, "DejaVu Serif", B_FONT_FAMILY_LENGTH + 1);
        else if (familyName.contains("Mono", false) != B_ERROR)
            strncpy(*fontFamily, "DejaVu Mono", B_FONT_FAMILY_LENGTH + 1);
        else {
            // This is the fallback font.
            strncpy(*fontFamily, "DejaVu Serif", B_FONT_FAMILY_LENGTH + 1);

This patch makes user can set fallback fonts. (Fallback fonts = user setting font in WebPositive's setting panel)

Attachments (2)

fallback.patch (5.1 KB ) - added by mt 9 years ago.
fallback2.patch (5.4 KB ) - added by mt 9 years ago.

Download all attachments as: .zip

Change History (19)

by mt, 9 years ago

Attachment: fallback.patch added

comment:1 by mt, 9 years ago

Has a Patch: set

comment:3 by stippi, 9 years ago

Thanks for the patch, it looks almost good: The WebCore layer changes are fine, but using them in the SettingsWindow class is a total layering violation. Everything must go through the WebKit API layer, no direct use of WebCore stuff in application code. It also means it happens in the wrong thread as a side effect. But if you move this code into the BWebSettings class, then it will be just fine.

comment:4 by stippi, 9 years ago

Does this patch somehow influence #6000 by any chance?

in reply to:  3 comment:5 by mt, 9 years ago

Replying to stippi: Thanks stippi for reviewing patch, I will correct it. Using BWebSettings in FontPlatformDataHaiku.cpp is ok ?

in reply to:  4 comment:6 by mt, 9 years ago

Replying to stippi:

Does this patch somehow influence #6000 by any chance?

I think this patch will be effective for some CJK web sites using CJK font not in Haiku.

For Ticket #6000, and other CJK font problem, I think we need enhancements bellow ;

  1. Change constant fallback fonts to user defined by Web+ setting. (This Ticket).
  2. Web+ font setting allow all CJK fonts for all styles (Standard, Serif, Sans Serif and Fixed font) like Bezilla.
  3. Add encoding menu to Web+, so we can set encoding when automatic encoding fails.

For No.2, I know Haiku will resolve it by Font-Overlay, but I think we can add like "Allow CJK font" or "Force using all CJK font" option to Web+ font settings and can set all CJK fonts like Bezilla when the option is on.

comment:7 by stippi, 9 years ago

No, using BWebSettings in FontPlatformDataHaiku.cpp is another layering violation. What I meant was to move the code you had in SettingsWindow to WebSettings or rather WebSettingsPrivate. It could be done automatically at the place where the regular font settings are applied.

comment:8 by stippi, 9 years ago

The layering is like this:

JavaScriptCore <- WebCore <- WebKit <- Application

So stuff in WebCore can't use stuff from WebKit (what's in the API folder) and the application can't use stuff from WebCore and so on.

comment:9 by mt, 9 years ago

Hi, I updated patch (move code to WebSettings), please review.

by mt, 9 years ago

Attachment: fallback2.patch added

comment:10 by stippi, 9 years ago

This looks pretty good now. I am going to apply it with some coding style fixes and then I want to see if setting the fall back fonts shouldn't happen in WebSettingsPrivate::apply(), when there is no WebCore::Settings object, i.e. the WebSettings object is the global default settings object. But as far as your patch is concerned, everything happens in the correct thread now and without layering violations. :-) Thanks a bunch!

comment:11 by leavengood, 9 years ago

Hi stippi,

Did you ever apply and commit this patch, or is there still work do to?

comment:12 by stippi, 9 years ago

Ah yes, sorry. I noticed on one of my machines that I applied this patch and worked on it. I noticed that for some reason, some fonts cannot really be picked in WebPositive. Some error with the font settings view. I was debugging the issue but stopped (or got distracted) without a solution. I'll see if I can complete this (hopefully next weekend, perhaps), but I don't know if the SVN server is already back up.

comment:13 by leavengood, 9 years ago

Well if you have the time to finish this up, that would be great. There isn't any hurry since I won't be around to work on Web+ this week. But I'd like to knock out as many of these bugs as I can while I have time over the holidays.

comment:14 by scottmc, 8 years ago

Was there any progress on this one in the recent build?

comment:15 by leavengood, 8 years ago

Owner: changed from stippi to leavengood
Status: newassigned

No. Right now I'm trying to update to the latest WebKit so I want to avoid introducing any other new changes for now. But I'll try to look at this after I'm done with the WebKit update, so I'll take ownership. We still have lots of things to improve in the port.

comment:16 by aldeck, 7 years ago

Priority: normalhigh

comment:17 by mshlyn, 6 years ago

Cc: linlongzhou@… added

comment:18 by pulkomandy, 6 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.