[Webkit-unassigned] [Bug 39654] Backslash is transcoded into yen sign even when non japanese font is specified

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39654





--- Comment #5 from Shinichiro Hamaji <hamaji at chromium.org>  2010-05-31 02:12:07 PST ---
Thanks for your review!

> In general, it isn't a great pattern to pass "this" to external functions from a constructor. The object may still be in some transitional state, and although it's valid 
> C++, it may catch the programmer by surprise:
> - subclass constructors haven't been invoked yet, and virtual methods table hasn't been swapped to final one yet;
> - a reference counted object can still have a refcount of 0, so taking a temporary RefPtr will destroy it from within the constructor;
> - post-constructor "init" functions that someone else wrote to avoid these problems haven't run yet;
> - etc.

Yeah, I totally agree. Fortunately, I could easily remove the "this" by using FontDescription instead of Font in this case.

> I'm still unclear on the "specified font" concept.

Basically, whenever a webpage specifies a font name by font-family or font, the font is considered "specified". If a non Japanese font is "specified", we guess the author of the web page intended to show backslashes even with Japanese encodings. If a generic font such as serif, sans-serif, etc. is specified, the font is considered "unspecified" because IE and Firefox choose locale specific fonts. We check only the first font family. I guess this is OK for now as most fonts would have U+005C. This limitation should be fixed if isSpecifiedFont will be used for other purpose. I doubt it will happen though.

I updated the comment on m_isSpecifiedFont. I hope the updated comment is clearer a bit. I'm not sure if the name "isSpecifiedFont" is good. Suggestions of a better name will be really appreciated.

> The test doesn't check multiple font names (e.g. '"some-windows-only-font", "Times"', or "some-windows-only-font", "MS PGothic").

Very good point. Actually, "Times, serif" was incorrect with the previous patch (the second font calls setIsSpecifiedFont). Now I fixed this issue and added some test cases. Thanks for catching this!

> Is "specified font" inherited from parent nodes?

Yes, just like font-family.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.