WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
140081
Font::primaryFontData() should return a reference
https://bugs.webkit.org/show_bug.cgi?id=140081
Summary
Font::primaryFontData() should return a reference
Antti Koivisto
Reported
2015-01-05 08:28:00 PST
It is not null
Attachments
patch
(64.63 KB, patch)
2015-01-05 09:23 PST
,
Antti Koivisto
no flags
Details
Formatted Diff
Diff
patch2
(64.67 KB, patch)
2015-01-05 09:27 PST
,
Antti Koivisto
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Antti Koivisto
Comment 1
2015-01-05 09:23:07 PST
Created
attachment 243977
[details]
patch
Antti Koivisto
Comment 2
2015-01-05 09:27:11 PST
Created
attachment 243978
[details]
patch2
Darin Adler
Comment 3
2015-01-05 09:37:35 PST
Comment on
attachment 243978
[details]
patch2 View in context:
https://bugs.webkit.org/attachment.cgi?id=243978&action=review
A tiny bit sad that something used this often got a little longer, but I suppose it’s strange that a font has a function that returns “a font”.
> Source/WebCore/platform/graphics/FontGlyphs.h:77 > + const FontData& primaryFontData(const FontDescription& description) { return *realizeFontDataAt(description, 0); }
Do we even need a function for this?
Antti Koivisto
Comment 4
2015-01-05 09:46:02 PST
(In reply to
comment #3
)
> Comment on
attachment 243978
[details]
> patch2 > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=243978&action=review
> > A tiny bit sad that something used this often got a little longer, but I > suppose it’s strange that a font has a function that returns “a font”.
Yeah. We might even want to rename Font to something like StyleFont (since it is essentially CSS font definition) and FontData to Font.
> Do we even need a function for this?
Not really. I'll get rid of it.
Myles C. Maxfield
Comment 5
2015-01-05 13:28:16 PST
See
https://bugs.webkit.org/show_bug.cgi?id=135290
Myles C. Maxfield
Comment 6
2015-01-05 13:31:44 PST
(In reply to
comment #4
)
> (In reply to
comment #3
) > > Comment on
attachment 243978
[details]
> > patch2 > > > > View in context: > >
https://bugs.webkit.org/attachment.cgi?id=243978&action=review
> > > > A tiny bit sad that something used this often got a little longer, but I > > suppose it’s strange that a font has a function that returns “a font”. > > Yeah. We might even want to rename Font to something like StyleFont (since > it is essentially CSS font definition) and FontData to Font. > > > Do we even need a function for this? > > Not really. I'll get rid of it.
I actually think that this is a good function to have. It lightens the cognitive load of code that calls this function, and I don't think there is any information lost between just using the name.
Antti Koivisto
Comment 7
2015-01-06 03:37:11 PST
https://trac.webkit.org/r177955
Antti Koivisto
Comment 8
2015-01-06 03:39:03 PST
(In reply to
comment #6
)
> I actually think that this is a good function to have. It lightens the > cognitive load of code that calls this function, and I don't think there is > any information lost between just using the name.
There were two functions with similar names. This one was used only in a few places internally in FontGlyphs and was really just creating confusion. Now "primary font data" is always a SimpleFontData.
Ahmad Saleem
Comment 9
2022-07-27 12:49:52 PDT
As per
Comment 07
, I did landed and there was no backout, I searched with bug number on Webkit Github. Can this be marked as "RESOLVED FIXED"? Thanks!
Ryosuke Niwa
Comment 10
2022-07-27 14:27:25 PDT
Yup, fixed.
Radar WebKit Bug Importer
Comment 11
2022-07-27 14:28:16 PDT
<
rdar://problem/97684797
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug