[webkit-reviews] review denied: [Bug 71765] Text dispappear when SVG font has no latin character : [Attachment 115096] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 15 06:33:46 PST 2011
Nikolas Zimmermann <zimmermann at kde.org> has denied Kenichi Ishibashi
<bashi at chromium.org>'s request for review:
Bug 71765: Text dispappear when SVG font has no latin character
https://bugs.webkit.org/show_bug.cgi?id=71765
Attachment 115096: Patch
https://bugs.webkit.org/attachment.cgi?id=115096&action=review
------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115096&action=review
Superb work bashi, I think this needs a split-up though, as its mixing things.
> Source/WebCore/css/CSSFontFace.cpp:108
> + if (Document* document = fontSelector->document()) {
> + if (document->settings() &&
document->settings()->fontLoadedEventEnabled() && document->domWindow()) {
> + RefPtr<Event> event = Event::create("webkitfontloaded", false,
false);
> + document->domWindow()->dispatchEvent(event, document);
> + }
> + }
Excellent approach! Though I think you should land this separated.
There are numerous of bugs reports regarding flakey SVG Fonts tests, that could
and should all be fixed, by listening to that new event.
Could you make a new bug report, find all bug reports for flakey svg fonts
(most come from chromium), mark them as depending of your master bug, fixup all
these tests, finish & land patch, then come back to this bug report.
> LayoutTests/svg/custom/svg-fonts-no-latin-glyph.html:27
> + document.documentElement.offsetTop;
> + window.addEventListener('webkitfontloaded', doTest);
I'd reverse the order here.
> LayoutTests/svg/custom/svg-fonts-no-latin-glyph.html:31
> + setTimeout(function() {
> + document.getElementById('result').innerHTML = 'TIMEOUT';
> + layoutTestController.notifyDone();
> + }, 500);
I'd just avoid that, it's confusing.
More information about the webkit-reviews
mailing list