JDK-8088205 : [Mac] WebView renders icons instead of letters on some sites
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 8,9
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-02-16
  • Updated: 2017-09-19
  • Resolved: 2016-12-01
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
8u152Fixed 9Fixed
Related Reports
Duplicate :  
Duplicate :  
Duplicate :  
Duplicate :  
Duplicate :  
Looks like font substitution problem: non-letter characters are rendered instead of normal letters and site becomes completely unreadable
Is a regression: no
Is platform-specific: yes (works fine on Linux) 

Steps to reproduce: open www.capitalone.com, www.ehow.com or nbcnews.com in webview
Actual result: can't read anything.
Approved to backport to 8u-dev for 8u152.

Requesting approval for backport to 8udev. The commited patch for 9dev applies cleanly to 8udev. I have tested the websites mentioned in the issue and they are fine along with the unit test.

changeset: ecec9ad850ce user: arajkumar date: Thu Dec 01 23:30:46 2016 +0530 8088205: [Mac] WebView renders icons instead of letters on some sites Reviewed-by: kcr, prr URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/ecec9ad850ce


Hi [~kcr], Addressed your review comments in http://cr.openjdk.java.net/~arajkumar/8088205/webrev.04 . Please take a look. Thanks.

Thank you [~prr] and [~kcr] for reviewing the fix. >>The (anonymous) inner class will hold a strong reference to the enclosing class and prevent garbage collection from >>happening. You should use an explicit static class instead. I will address your concern in a new webrev. >> 2. In your current patch, you list ���vadim��� as a reviewer rather than ���prr���, so don���t forget to fix this. I use mercurial bookmarks to manage multiple patches and I run `webrev.ksh -r bookmarkname~1` to generate webrev. Looks like webrev tool takes few old commit logs while generating the pretty webrev patch. If you look at the rt.patch, it doesn't have any commit log. Looks like a webrev tool bug.

Two comments: 1. I believe the following disposer record will be ineffective: + Disposer.addRecord(this, new DisposerRecord() { + public synchronized void dispose() { + if (cgFontRef != 0) { + OS.CFRelease(cgFontRef); + cgFontRef = 0; + } + } + }); The (anonymous) inner class will hold a strong reference to the enclosing class and prevent garbage collection from happening. You should use an explicit static class instead. 2. In your current patch, you list ���vadim��� as a reviewer rather than ���prr���, so don���t forget to fix this.

That looks better. "+1".

Hi [~prr], I have uploaded a new patch[1] which addresses your concerns. Please take a look. [1] http://cr.openjdk.java.net/~arajkumar/8088205/webrev.03

[~prr], Thanks for your detailed review comments. I referred other platform implementation (DWFontFile and FTFontFile), * Implementation for Direct Write creates fontFace handle based on fileName if it's type is Embedded Font. (Refer DWFontFile.createFontFace and DWFontFile.createEmbeddedFontFace methods) * Implementation for Free Type creates fontFace handle based on fileName and fontIndex regardless of it's nature (embedded or installed). I could see in both platforms(DW and FT) fontFace is owned by it's corresponding FontFile instance(DWFontFile / FTFontFile), but it is not the case for CTFontFile.

This looks wrong to me. Without this fix when we go to create a strike we use the Postscript name to locate it. This is exactly what you want to do for all the installed fonts. And ones that have been registered via Font.loadFont(). The fix proposes to make a font out of a file for this because it is what webkit needs for WOFF fonts (as far as I can see). By going off to get the file name I don't know if this is going to work for dfonts. Also I don't understand the fall back to TTC path which seems to essentially retain the same code that supposedly doesn't work for WOFF. Also for the cases where we have a reference to a simple .TTF font that is installed on local disk it appears to treat that as a data source from which to create a wholly new font - for every strike - !. This is wrong and incredibly inefficient. There needs to be a code path which does something like this but *only* for the cases that are affected. We do already have working code for the embedded fonts rendered by webkit on other platforms and we should be trying to follow through that path and keep this isolated. Webkit fonts must not show up in the "list of fonts" seen by an app. I am not saying this does that .. but I believe we have code that keeps these from being enumerated and part of testing this fix needs to be to make sure these temp fonts don't pollute that space but are available to webkit.

Thanks [~kcr]. http://cr.openjdk.java.net/~arajkumar/8088205/webrev.02 I have done a simple refactoring for correctness. fontRef creation shouldn't be dependent on familyName, if familyName is empty or null still it should able to create a fontRef using given font data.

This looks good to me. Phil should review it, too. +1

Thanks [~kcr]. I have incorporated your review comments in http://cr.openjdk.java.net/~arajkumar/8088205/webrev.01. Please take a look.

I ran this, and it looks like it fixes the problem, although I didn't do exhaustive testing. One thing, though, is that it deviates from the code pattern that was done for the native font code (and accessibility code), which keeps all logic in java and has each native method just be a 1-1 call to a single native method. So instead of doing the whole thing in one method, the approach consistent with the design would be to have methods for each of the following in coretext.c and native methods to expose them in the OS class: CFURLCreateWithFileSystemPath() CGDataProviderCreateWithURL() CGFontCreateWithDataProvider() CTFontCreateWithGraphicsFont() CFRelease() CTFontCreateWithName() Some of these already exist.


WebKit on OSX uses the below code snippet to load Web fonts, ========================================================= #if CORETEXT_WEB_FONTS RetainPtr<CTFontDescriptorRef> fontDescriptor = adoptCF(CTFontManagerCreateFontDescriptorFromData(bufferData.get())); if (!fontDescriptor) return nullptr; return std::make_unique<FontCustomPlatformData>(fontDescriptor.get()); #else RetainPtr<CGDataProviderRef> dataProvider = adoptCF(CGDataProviderCreateWithCFData(bufferData.get())); RetainPtr<CGFontRef> cgFontRef = adoptCF(CGFontCreateWithDataProvider(dataProvider.get())); if (!cgFontRef) return nullptr; return std::make_unique<FontCustomPlatformData>(cgFontRef.get()); #endif ========================================================= i.e. it uses either CGFontCreateWithDataProvider or CTFontManagerCreateFontDescriptorFromData to get the CTFontRef. But JavaFX uses CTFontManagerRegisterFontsForURL, it doesn't give any handle to font instead only registration happens. Registration is not proper with the provided faulty "Thin.ttf or Thin.woff" without proper family name. I did a small POC using CGFontCreateWithDataProvider and it seems to be working.

OSX "fontbook" app also shows glyphs as "?", it says some error in the give font "Thin.tff"

Could reproduce the issue using javafx label. Refer the CustomFontApp.tar.bz2, Load external Thin.woff font using Font.loadFont(..), Set font family using "-fx-font-family: Proxima Nova" css style. Issue is seen only in OSX.

Attached a reduced test case to reproduce the issue. Issue is related to Proxima Web Font rendering.

Evergreen found that bug on cnet.com and flickr. RULE "sites/cnet.com" any any RULE "sites/flickr.com" any any

Yes, I saw that. I agree it's ugly... (I can look into it after ZBB in case there's a spare time)

Btw, I still agree that we should defer this. But if you want to see how bad it is, go to the following site in WebView on Mac: http://flickr.com/

no needs to add deferral label for non regression case

I'm reproducing this since at least 8u20. I went to google news digest and navigated to top ~20 news/media resources. Discovered one where the bug is reproduced: cnet.com. So, roughly it's reproducible in 5% of cases where text appears unreadable. Can't guess what's the reason. From the CoreText message in the above comments it seems that WebView/WebKit fails to get a font name. Mac only specific. Unfortunately, I have to request deferral to 9 due to lack of resources.

not 8u60 regression, do we want to retarget to 9. Anton?

Can't reproduce it on Windows/Linux with 8u-dev, but on Mac it's still there. I'm getting the following message in console: [java] 2015-04-23 17:36:54.880 java[53140:d07] CoreText performance note: Client called CTFontCreateWithName() using name "" and got font with PostScript name "--unknown-1--". For best performance, only use PostScript names when calling this API. Not sure if it is related.

Remove 8u45 from affects version, since it was discovered in 8u31.

I saw this yesterday during my testing of the merged WebKit (so it is still an issue in 9) when I went to http://flickr.com/ Raising to P3 since this is pretty serious rendering bug.

RULE sites/capitalone.com any any RULE sites/ehow.com any any RULE sites/nbcnews.com any any