Bug 112651

Summary:

SVG image container size can survive cached reloads

Product:

WebKit

Reporter:

Philip Rogers <pdr>

Component:

SVG

Assignee:

Philip Rogers <pdr>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, d-r, fmalita, rniwa, schenney, stephen.bannasch, webkit.review.bot, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Testcase	none
Separate SVG image size and container size	schenney: review-, buildbot: commit-queue-
Update per reviewer comments	none

Philip Rogers

Reported 2013-03-18 20:41:27 PDT

Created attachment 193719 [details] Testcase SVG image container sizes can survive cached reloads if the container size is queried before the image is rendered. See the attached testcase for a minimized reproduction.

Attachments
Testcase (969 bytes, text/html) 2013-03-18 20:41 PDT, Philip Rogers	no flags	Details
Separate SVG image size and container size (7.97 KB, patch) 2013-03-18 22:54 PDT, Philip Rogers	schenney: review- buildbot: commit-queue-	Details Formatted Diff Diff
Update per reviewer comments (7.87 KB, patch) 2013-03-19 10:39 PDT, Philip Rogers	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Philip Rogers

Comment 1 2013-03-18 22:54:27 PDT

Created attachment 193736 [details] Separate SVG image size and container size

Build Bot

Comment 2 2013-03-19 03:16:36 PDT

Comment on attachment 193736 [details] Separate SVG image size and container size Attachment 193736 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17066866 New failing tests: fast/css/sticky/sticky-both-sides.html fast/css/sticky/inline-sticky.html

Stephen Chenney

Comment 3 2013-03-19 06:46:13 PDT

Comment on attachment 193736 [details] Separate SVG image size and container size View in context: https://bugs.webkit.org/attachment.cgi?id=193736&action=review The wk2 test is a flake, so don't worry about that. Otherwise I'm basically happy, but it could do with a little polish. > Source/WebCore/svg/graphics/SVGImage.h:54 > + virtual IntSize size() const OVERRIDE { return m_intrinsicSize; } Can this now be renamed "instrinsicSize" to make it clear what this size means? > LayoutTests/svg/as-image/svg-container-size-after-reload.html:2 > +<html> This kind of testing looks to be in the same format as the svg dynamic-tests or, more simply, the js pre/post test harness. Your loading some content and running some script and checking the values. Please try to convert to that format.

Philip Rogers

Comment 4 2013-03-19 10:39:14 PDT

Created attachment 193855 [details] Update per reviewer comments

Philip Rogers

Comment 5 2013-03-19 10:42:50 PDT

(In reply to comment #3) > (From update of attachment 193736 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193736&action=review > > The wk2 test is a flake, so don't worry about that. Otherwise I'm basically happy, but it could do with a little polish. > > > Source/WebCore/svg/graphics/SVGImage.h:54 > > + virtual IntSize size() const OVERRIDE { return m_intrinsicSize; } > > Can this now be renamed "instrinsicSize" to make it clear what this size means? I think this will unnecessarily increase code complexity. SVGImage is the only subclass of Image that has the intrinsic distinction. > > > LayoutTests/svg/as-image/svg-container-size-after-reload.html:2 > > +<html> > > This kind of testing looks to be in the same format as the svg dynamic-tests or, more simply, the js pre/post test harness. Your loading some content and running some script and checking the values. Please try to convert to that format. Agreed. This ended up making the test slightly smaller and more maintainable.

Stephen Chenney

Comment 6 2013-03-19 11:05:31 PDT

(In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 193736 [details] [details]) > > Can this now be renamed "instrinsicSize" to make it clear what this size means? > > I think this will unnecessarily increase code complexity. SVGImage is the only subclass of Image that has the intrinsic distinction. You're right. Disregard my comment.

Stephen Chenney

Comment 7 2013-03-19 11:07:14 PDT

Comment on attachment 193855 [details] Update per reviewer comments Great. R=me.

WebKit Review Bot

Comment 8 2013-03-19 11:41:40 PDT

Comment on attachment 193855 [details] Update per reviewer comments Clearing flags on attachment: 193855 Committed r146227: <http://trac.webkit.org/changeset/146227>

WebKit Review Bot

Comment 9 2013-03-19 11:41:44 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.