Bug 79811 – Reimplement pathFromFont() in SimpleFontDataMac.mm

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Text

[Ned Holbrook]

It certainly can't hurt to fix this debugging function.

[Ned Holbrook]

Created attachment 129296 [details]
Proposed changes.

Comment on attachment 129296 [details]
Proposed changes.

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

OK. I think you could have just as well removed this function and changed its callers.

> Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm:85
> +static NSString* pathFromFont(NSFont* font)

Spaces should go before the star in Objective-C objects (I know this was already wrong in some of this code).

> Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm:87
> +    return [[[font fontDescriptor] objectForKey:(NSString*)kCTFontURLAttribute] path];

Needs a space before the *.

[Ned Holbrook]

Created attachment 129299 [details]
Changes per review.

I fixed the spaces, but I could always remove it if you think it's not valuable.

[WebKit Review Bot]

Comment on attachment 129299 [details]
Changes per review.

Clearing flags on attachment: 129299

Committed r109137: <http://trac.webkit.org/changeset/109137>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Klein]

This breaks the Chromium Mac builders, I think because kCTFontURLAttribute is not available on Leopard.  Sample failure:

http://build.chromium.org/p/chromium.webkit/builders/Mac%20Builder%20%28dbg%29/builds/4938/steps/compile/logs/stdio

Given that this is just for debugging I'd prefer to simply roll this out for now, please speak up if you disagree.

(In reply to comment #6)
> This breaks the Chromium Mac builders, I think because kCTFontURLAttribute is not available on Leopard.

Sorry about that.

>  Sample failure:
> 
> http://build.chromium.org/p/chromium.webkit/builders/Mac%20Builder%20%28dbg%29/builds/4938/steps/compile/logs/stdio
> 
> Given that this is just for debugging I'd prefer to simply roll this out for now, please speak up if you disagree.

Yes, feel free to revert this change.

[Adam Klein]

Rolled out in http://trac.webkit.org/changeset/109146

[Ned Holbrook]

Leopard? Hmm. Given the earlier comments, what say I just rip it out altogether?

I’m still ok with that. Or just not touching it.

[Ned Holbrook]

Okay, I'll let sleeping dogs lie.