[Webkit-unassigned] [Bug 144640] Handle meta viewport in HTMLPreloadScanner

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144640

--- Comment #8 from Darin Adler <darin at apple.com> ---
Comment on attachment 252627
  --> https://bugs.webkit.org/attachment.cgi?id=252627
Patch

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

I’m not happy with the testing strategy here.

> 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);
> +}

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?).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150510/ef908e69/attachment.html>