Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(77)

Issue 983973004: Provide user friendly messages for OTS parsing of fonts (Closed)

Created:
5 years, 9 months ago by h.joshi
Modified:
5 years, 6 months ago
Reviewers:
jungshik at Google, eae
CC:
blink-reviews, krit, tyoshino+watch_chromium.org, pdr+graphicswatchlist_chromium.org, Dominik Röttsches, blink-reviews-css, dshwang, ed+blinkwatch_opera.com, Justin Novosad, jbroman, danakj, dglazkov+blink, Rik, apavlov+blink_chromium.org, gavinp+loader_chromium.org, darktears, f(malita), Stephen Chennney, Nate Chapin, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Provide user friendly messages for OTS parsing of fonts Add feature to provide user (font-developer) friendly messages for used web font parsing errors. Few new error/warnings related to font parsing and tables are added in https://codereview.chromium.org/966703002/. BUG=97467 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197551

Patch Set 1 #

Patch Set 2 : Rebased patch #

Patch Set 3 : Updating Test Expectation #

Patch Set 4 : Rebase patch #

Patch Set 5 : Code update for test expectation #

Total comments: 4

Patch Set 6 : Comment fixes #

Total comments: 4

Patch Set 7 : Correcting Method call #

Total comments: 10

Patch Set 8 : Remove unused variable #

Patch Set 9 : Comment fix-adding error string to other NULL return statements #

Patch Set 10 : Updating Test Expectation #

Patch Set 11 : Rebase patch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -33 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 2 chunks +10 lines, -1 line 0 comments Download
M Source/core/css/BinaryDataFontFaceSource.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/BinaryDataFontFaceSource.cpp View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/css/FontFace.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/FontFace.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/FontLoader.cpp View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/fetch/FontResource.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/fetch/FontResource.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/fonts/FontCustomPlatformData.h View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/fonts/FontCustomPlatformData.cpp View 1 2 3 4 5 7 1 chunk +5 lines, -2 lines 0 comments Download
M Source/platform/fonts/opentype/OpenTypeSanitizer.h View 1 2 3 4 5 6 7 2 chunks +14 lines, -21 lines 0 comments Download
M Source/platform/fonts/opentype/OpenTypeSanitizer.cpp View 1 2 3 4 5 6 7 8 3 chunks +59 lines, -3 lines 0 comments Download

Messages

Total messages: 32 (12 generated)
h.joshi
Pls review
5 years, 7 months ago (2015-05-12 04:21:33 UTC) #3
jungshik at Google
Just had a brief look. A couple of questions. BTW, I have another question about ...
5 years, 7 months ago (2015-05-12 20:22:48 UTC) #5
h.joshi
@Jungshik: Made suggested changes BTW, I have another question about the console message: Do we ...
5 years, 7 months ago (2015-05-14 04:52:16 UTC) #6
jungshik at Google
Thank you for the update. A couple of nit-like comments below. As for the localization, ...
5 years, 7 months ago (2015-05-15 19:17:21 UTC) #8
h.joshi
Correcting method "setParsingError" call due to which in few places uninitialized variable was present. https://codereview.chromium.org/983973004/diff/160001/Source/platform/fonts/FontCustomPlatformData.cpp ...
5 years, 7 months ago (2015-05-18 05:58:28 UTC) #10
jungshik at Google
Sorry that I had a few draft comments that I thought I had sent your ...
5 years, 6 months ago (2015-06-01 23:22:19 UTC) #11
h.joshi
removal of "isParsingError" method and related variables were done already in previous patch. Not sure ...
5 years, 6 months ago (2015-06-04 15:19:17 UTC) #12
jungshik at Google
Thanks. LGTM
5 years, 6 months ago (2015-06-08 18:57:42 UTC) #13
h.joshi
@Jungshik: Thank you
5 years, 6 months ago (2015-06-10 03:30:16 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/983973004/240001
5 years, 6 months ago (2015-06-10 03:30:42 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/34988)
5 years, 6 months ago (2015-06-10 03:37:22 UTC) #18
h.joshi
@Emil: Pls review Getting presubmit OWNER LGTM error for "Source/core" file.
5 years, 6 months ago (2015-06-10 03:48:32 UTC) #19
h.joshi
@Emil: Pls review
5 years, 6 months ago (2015-06-12 01:42:06 UTC) #20
h.joshi
@Emil: Pls review
5 years, 6 months ago (2015-06-17 02:25:52 UTC) #21
eae
LGTM
5 years, 6 months ago (2015-06-20 00:25:53 UTC) #22
h.joshi
Thank you Emil
5 years, 6 months ago (2015-06-20 02:05:27 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/983973004/260001
5 years, 6 months ago (2015-06-20 02:06:09 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/35926)
5 years, 6 months ago (2015-06-20 02:12:17 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/983973004/280001
5 years, 6 months ago (2015-06-22 03:55:45 UTC) #31
commit-bot: I haz the power
5 years, 6 months ago (2015-06-22 05:16:45 UTC) #32
Message was sent while issue was closed.
Committed patchset #11 (id:280001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197551

Powered by Google App Engine
This is Rietveld 408576698