RESOLVED FIXED81630
"Attempt to insert nil value " exception when calling attributed string APIs on content with a custom font
https://bugs.webkit.org/show_bug.cgi?id=81630
Summary "Attempt to insert nil value " exception when calling attributed string APIs ...
Adele Peterson
Reported 2012-03-19 22:17:02 PDT
"Attempt to insert nil value " exception when calling attributed string APIs on content with a custom font
Attachments
patch (13.79 KB, patch)
2012-03-19 22:21 PDT, Adele Peterson
no flags
patch (14.07 KB, patch)
2012-03-20 13:14 PDT, Adele Peterson
ap: review+
updated the test (1.56 KB, patch)
2012-03-20 14:35 PDT, Adele Peterson
mitz: review+
Adele Peterson
Comment 1 2012-03-19 22:21:47 PDT
Created attachment 132755 [details] patch I'm not sure if the way I'm referencing the font in the TestWebKitAPI project file is kosher. If it is not, I can just check in another copy of Ahem.
Alexey Proskuryakov
Comment 2 2012-03-20 10:06:08 PDT
Comment on attachment 132755 [details] patch Can this just use a layout test instead? We expose these methods through textInputController in DumpRenderTree. WebKitTestRunner is currently more limited, but it will definitely support this eventually, too.
Adele Peterson
Comment 3 2012-03-20 10:32:09 PDT
I do have a layout test that works in DRT, but since I couldn't get it to work in WebKitTestRunner, even while trying to add the feature to textInputController, I switched to making an API test. I couldn't figure out how to call attributedSubstringForProposedRange from WebKitTestRunner/InjectedBundle/TextInputController.cpp. Do you think that is possible? (In reply to comment #2) > (From update of attachment 132755 [details]) > Can this just use a layout test instead? We expose these methods through textInputController in DumpRenderTree. WebKitTestRunner is currently more limited, but it will definitely support this eventually, too.
Alexey Proskuryakov
Comment 4 2012-03-20 10:53:03 PDT
I see, so the issue here is that we need to do the conversion in another process. That won't work with textInputController indeed. If you have attributedSubstringForProposedRange implemented in WTR, I'd encourage you to see if you can land that and unskip some of the tests that are currently skipped in mac-wk2. Patch looks good to me, but I don't know the answer to your question about Ahem.
Adele Peterson
Comment 5 2012-03-20 11:03:16 PDT
(In reply to comment #4) > I see, so the issue here is that we need to do the conversion in another process. That won't work with textInputController indeed. > > If you have attributedSubstringForProposedRange implemented in WTR, I'd encourage you to see if you can land that and unskip some of the tests that are currently skipped in mac-wk2. No, I just have an empty implementation. > > Patch looks good to me, but I don't know the answer to your question about Ahem.
mitz
Comment 6 2012-03-20 11:51:40 PDT
(In reply to comment #1) > Created an attachment (id=132755) [details] > patch > > I'm not sure if the way I'm referencing the font in the TestWebKitAPI project file is kosher. If it is not, I can just check in another copy of Ahem. Please give TestWebKitAPI its own copy.
Adele Peterson
Comment 7 2012-03-20 13:14:26 PDT
Created attachment 132884 [details] patch Updated with a new copy of Ahem for TestWebKitAPI
Adele Peterson
Comment 8 2012-03-20 14:06:15 PDT
mitz
Comment 9 2012-03-20 14:14:22 PDT
Comment on attachment 132884 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=132884&action=review > Tools/TestWebKitAPI/Tests/mac/AttributedString.mm:88 > + EXPECT_TRUE([[attrString string] isEqual:@"Lorem"]); You could have used EXPECT_WK_STREQ here. When it fails, prints the actual string (as opposed to just the fact that it’s not the expected one).
Adele Peterson
Comment 10 2012-03-20 14:35:11 PDT
Created attachment 132902 [details] updated the test
mitz
Comment 11 2012-03-20 14:37:18 PDT
Comment on attachment 132902 [details] updated the test View in context: https://bugs.webkit.org/attachment.cgi?id=132902&action=review > Tools/ChangeLog:5 > + Reviewed by . Me!
Adele Peterson
Comment 12 2012-03-20 14:43:22 PDT
That's what I had in there, but I didn't want to be presumptuous :) http://trac.webkit.org/changeset/111443 (In reply to comment #11) > (From update of attachment 132902 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132902&action=review > > > Tools/ChangeLog:5 > > + Reviewed by . > > Me!
Note You need to log in before you can comment on or make changes to this bug.