[Webkit-unassigned] [Bug 64883] Zoom only scales frames' content, not the frames themselves.

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Julien Chaffraix <jchaffraix at webkit.org>  2012-05-17 09:58:03 PST ---
(From update of attachment 141859)
View in context: https://bugs.webkit.org/attachment.cgi?id=141859&action=review

> Source/WebCore/rendering/RenderFrameSet.cpp:218
> +            int fixedLen = snapSizeToPixel(grid[i].value() * style()->effectiveZoom(), totalFixed);

Emil, you suggested to use snapSizeToPixel but it seems misguided as snapSizeToPixel is used for converting sizes not 1-dimensional coordinates AFAICT. It won't give any guarantees on whether we account for all the available space or spread the error for that matter. This is unfortunately a common issue (tables are also impacted).

More to the point, snapSizeToPixel forces us to convert the values to FractionalUnits and then back to int which seems wrong and wasteful. I would just go with lroundf or ceilf in this case.

> LayoutTests/ChangeLog:11
> +        * fast/frames/frame-set-zoom-page-expected.html: Added.
> +        * fast/frames/frame-set-zoom-page.html: Added.

This covers only rows. Could we have a similar one for cols as they are also impacted by this change?

> LayoutTests/fast/frames/frame-set-zoom-page.html:4
> +  <title> FrameSet Zoom Test</title>

This title doesn't add much, it should be dropped.

> LayoutTests/fast/frames/frame-set-zoom-page.html:14
> +</head>

Ideally we need in the output:
* Bug id, bonus points if it's clickable.
* Bug title.
* What are your expectations for the test to pass or fail? (here it looks like: the frameset size should increase to account for the zoom)
* Manual testing / warning when not run under DRT (if applicable)

If it cannot be in the output (which I feared because of the <frameset>), it should be in the markup.

> LayoutTests/fast/frames/frame-set-zoom-page.html:15
> +	<frameset rows="50,*" style="background-color: red">

I don't understand the need for the red background. Currently we don't account for the zoom factor so the background will not be seen. In my simple mind, 'red' means failure and here it doesn't seem to equate that.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.