Bug 65197 - Add a generic pictograph font family
Summary: Add a generic pictograph font family
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: mitz
URL:
Keywords: InRadar
Depends on:
Blocks: 229131
  Show dependency treegraph
 
Reported: 2011-07-26 11:39 PDT by mitz
Modified: 2021-08-16 02:10 PDT (History)
3 users (show)

See Also:


Attachments
Add -webkit-pictograph generic family, WebKit2 API and WebKit1 Mac and Windows API for setting pictograph font family (48.21 KB, patch)
2011-07-26 11:56 PDT, mitz
no flags Details | Formatted Diff | Diff
Add -webkit-pictograph generic family, WebKit2 API and WebKit1 Mac and Windows API for setting pictograph font family (48.97 KB, patch)
2011-07-26 12:56 PDT, mitz
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2011-07-26 11:39:59 PDT
Add a generic pictograph font family
Comment 1 Radar WebKit Bug Importer 2011-07-26 11:41:13 PDT
<rdar://problem/9842889>
Comment 2 mitz 2011-07-26 11:56:20 PDT
Created attachment 102036 [details]
Add -webkit-pictograph generic family, WebKit2 API and WebKit1 Mac and Windows API for setting pictograph font family
Comment 3 WebKit Review Bot 2011-07-26 11:59:44 PDT
Attachment 102036 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/win/WebPreferences.cpp:656:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.cpp:663:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.h:94:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.h:97:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.h:98:  The parameter name "family" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 5 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Anders Carlsson 2011-07-26 12:07:17 PDT
Comment on attachment 102036 [details]
Add -webkit-pictograph generic family, WebKit2 API and WebKit1 Mac and Windows API for setting pictograph font family

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

Will the test work on earlier versions of Mac OS X?

> Source/WebKit/mac/WebView/WebView.mm:1512
> +//    settings->setPictographFontFamily([preferences pictographFontFamily]);

This should either be uncommented or removed.
Comment 5 mitz 2011-07-26 12:18:05 PDT
(In reply to comment #4)
> (From update of attachment 102036 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102036&action=review
> 
> Will the test work on earlier versions of Mac OS X?

No.

> 
> > Source/WebKit/mac/WebView/WebView.mm:1512
> > +//    settings->setPictographFontFamily([preferences pictographFontFamily]);
> 
> This should either be uncommented or removed.

It should be uncommented. I mistakenly commented it after running the tests and before uploading the patch.
Comment 6 mitz 2011-07-26 12:56:11 PDT
Created attachment 102041 [details]
Add -webkit-pictograph generic family, WebKit2 API and WebKit1 Mac and Windows API for setting pictograph font family
Comment 7 WebKit Review Bot 2011-07-26 12:59:34 PDT
Attachment 102041 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebKit/win/WebPreferences.cpp:656:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.cpp:663:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.h:94:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.h:97:  Extra space after ( in function call  [whitespace/parens] [4]
Source/WebKit/win/WebPreferences.h:98:  The parameter name "family" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 5 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 mitz 2011-07-26 13:25:22 PDT
Committed r91777. <http://trac.webkit.org/r91777>
Comment 9 Adrienne Walker 2011-07-26 16:31:05 PDT
Committed r91798: <http://trac.webkit.org/changeset/91798>
Comment 10 Adrienne Walker 2011-07-26 16:32:10 PDT
(In reply to comment #9)
> Committed r91798: <http://trac.webkit.org/changeset/91798>

mitz: Could you take a look at what Chromium output and see if that looks reasonable?
Comment 11 mitz 2011-07-26 16:34:14 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > Committed r91798: <http://trac.webkit.org/changeset/91798>
> 
> mitz: Could you take a look at what Chromium output and see if that looks reasonable?

Looks OK.
Comment 12 Adrienne Walker 2011-07-26 16:35:10 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Committed r91798: <http://trac.webkit.org/changeset/91798>
> > 
> > mitz: Could you take a look at what Chromium output and see if that looks reasonable?
> 
> Looks OK.

Thanks! :)
Comment 13 Frédéric Wang (:fredw) 2020-03-26 06:47:48 PDT
Comment on attachment 102041 [details]
Add -webkit-pictograph generic family, WebKit2 API and WebKit1 Mac and Windows API for setting pictograph font family

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

@mitz: I found this old bug while trying to check how to implement font-family: math in Chromium (incidentally for WebKit it is bug 209595) and I think there was a mistake in this patch. See comments below.

> Source/WebCore/css/CSSFontSelector.cpp:406
> +        genericFamily = settings->pictographFontFamily();

This only uses raw string comparison of the font-family name. This does not even require "-webkit-pictograph" to be parsed as a CSSValueWebkitPictograph, so your test can still work despite the mistake. Actually I'm removing dead code in https://chromium-review.googlesource.com/c/chromium/src/+/2119545 and it still passes.

> Source/WebCore/css/CSSValueKeywords.in:107
> +-webkit-pictograph

I believe CSSValueWebkitPictograph is actually never parsed. This is because consumeGenericFamily always only accepts values between "serif" and "-webkit-body" and you added "-webkit-pictograph" *after* them. I guess the only possible place where CSSValueWebkitPictograph is generated is WebCore/css/CSSComputedStyleDeclaration.cpp although that didn't seem to be triggered by your test when I tried in Chromium...