<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED FIXED - Handle meta viewport in HTMLPreloadScanner"
href="https://bugs.webkit.org/show_bug.cgi?id=144640#c8">Comment # 8</a>
on <a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED FIXED - Handle meta viewport in HTMLPreloadScanner"
href="https://bugs.webkit.org/show_bug.cgi?id=144640">bug 144640</a>
from <span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=252627&action=diff" name="attach_252627" title="Patch">attachment 252627</a> <a href="attachment.cgi?id=252627&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=252627&action=review">https://bugs.webkit.org/attachment.cgi?id=252627&action=review</a>
I’m not happy with the testing strategy here.
<span class="quote">> Source/WebCore/html/parser/HTMLPreloadScanner.cpp:373
> +bool testPreloadScannerViewportSupport(Document* document)
> +{
> + ASSERT(document);
> + HTMLParserOptions options(*document);
> + HTMLPreloadScanner scanner(options, document->url());
> + HTMLResourcePreloader preloader(*document);
> + scanner.appendToEnd(String("<meta name=viewport content='width=400'>"));
> + scanner.scan(preloader, *document);
> + return (document->viewportArguments().width == 400);
> +}</span >
This is not how we expose our internals for testing. We don’t just embed individual tests in the code like this. Lets think about how to do this in a way that matches the rest of our strategy.
A test like this one would make sense in Tools/TestWebKitAPI/Tests/WebCore; there are plenty of others like that there.
Internals exposes things so they can be tested, it doesn’t expose tests already compiled in. We could expose something testable in internals that lets the JavaScript test machinery drive the preload scanner and check its results.
What we want even more is a test that shows the actual end-user effect of this is, testing the whole thing and not just a unit test. But I don’t understand what that effect is. What is the value of calling document.processViewport during preloading? Is it correct to do that at the time of preload scanning? Lets make it so the test machinery can test that real desired effect (earlier correct viewport maybe?).</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>