Bug 139864 - Generic font code should not know about SVG font missing glyph
Summary: Generic font code should not know about SVG font missing glyph
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 139905
Blocks:
  Show dependency tree / graph
 
Reported: 2014-12-22 09:21 PST by Antti Koivisto
Modified: 2014-12-23 09:11 PST (History)
13 users (show)

See Also:

Attachments
patch (15.28 KB, patch)
2014-12-22 09:53 PST, Antti Koivisto 		kling: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2014-12-22 09:21:42 PST 
It is an SVG font concept and should be handled in SVG code.
Comment 1 Antti Koivisto 2014-12-22 09:53:16 PST 
Created attachment 243625 [details]
patch
Comment 2 Myles C. Maxfield 2014-12-22 10:54:07 PST 
Comment on attachment 243625 [details]
patch

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

r=me

> Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp:295
> +    const SimpleFontData* primaryFontData = font.primaryFont();

This logic seems completely busted (of always pulling out the primary font). Luckily SVG fonts as a whole are going away...
Comment 3 Andreas Kling 2014-12-22 10:55:41 PST 
Comment on attachment 243625 [details]
patch

I trust Myles on this. r+
Comment 4 Myles C. Maxfield 2014-12-22 10:57:34 PST 
Seems like this means that we'll never show the SVG font's missing-glyph. I think that's okay, but other people on the team might have opinions.
Comment 5 Antti Koivisto 2014-12-22 11:09:47 PST 
(In reply to comment #4)
> Seems like this means that we'll never show the SVG font's missing-glyph. I
> think that's okay, but other people on the team might have opinions.

There shouldn't be any behavior change, the missing glyphs work as before (and there are tests). They are just resolved bit differently.
Comment 6 Antti Koivisto 2014-12-22 11:16:00 PST 
https://trac.webkit.org/r177637
Comment 7 Brent Fulgham 2014-12-22 12:05:41 PST 
This breaks the Windows build:

         UniscribeController.cpp
     1>..\platform\graphics\win\SimpleFontDataCGWin.cpp(91): error C2027: use of undefined type 'WebCore::GlyphPageTreeNode'
                 c:\cygwin\home\buildbot\slave\win-debug\build\source\webcore\platform\graphics\GlyphPage.h(43) : see declaration of 'WebCore::GlyphPageTreeNode'
     1>..\platform\graphics\win\SimpleFontDataCGWin.cpp(91): error C2227: left of '->page' must point to class/struct/union/generic type
                 type is 'unknown-type'
     1>..\platform\graphics\win\SimpleFontDataCGWin.cpp(91): error C3861: 'getRootChild': identifier not found
         ScrollbarThemeMock.cpp
         FormData.cpp

I think we're just missing a #include in the win/SimpleFontDataCGWin.cpp. I'm testing this now and will check in a fix shortly if it works.
Comment 8 Brent Fulgham 2014-12-22 12:10:31 PST 
Build fix (Windows);

https://trac.webkit.org/r177644.
Comment 9 Antti Koivisto 2014-12-22 12:11:19 PST 
Build fix was https://trac.webkit.org/r177642
Comment 10 Csaba Osztrogonác 2014-12-23 09:11:42 PST 
(In reply to comment #6)
> https://trac.webkit.org/r177637

It made 3 tests crash on EFL and GTK, here is the new bug report:
https://bugs.webkit.org/show_bug.cgi?id=139905