Bug 103304 - REGRESSION: SVG setAttribute does not force a layout/repaint
Summary: REGRESSION: SVG setAttribute does not force a layout/repaint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philip Rogers
URL:
Keywords:
: 106923 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-11-26 15:24 PST by Philip Rogers
Modified: 2013-01-16 22:42 PST (History)
9 users (show)

See Also:


Attachments
Testcase (1.13 KB, text/html)
2012-11-26 15:24 PST, Philip Rogers
no flags Details
Invalidate SVG width on width attribute changes. (5.47 KB, patch)
2012-12-01 23:36 PST, Philip Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2012-11-26 15:24:50 PST
Created attachment 176088 [details]
Testcase

Setting width via setAttribute does not force a layout/repaint whereas setting it via style does.

Original bug: http://code.google.com/p/chromium/issues/detail?id=162628
Comment 1 Alexey Proskuryakov 2012-11-27 11:25:29 PST
Is it known when this regressed? It fails even in Safari 6.0.2.
Comment 2 Dirk Schulze 2012-11-27 14:09:20 PST
Philip, does this just happen with inline SVG? Does it just happen with SVG root elements? I think we have a lot of tests for pure SVG covering this. So it might be a problem between HTML and SVG.
Comment 3 Philip Rogers 2012-11-28 16:30:53 PST
I think this may be caused by the supremely confusing fact that style="width:x" and width="x" are different. For the most part, this affects the svg/html boundary and is present on <svg>.
Comment 4 Patrick Ruzand 2012-11-29 02:00:06 PST
(In reply to comment #1)
> Is it known when this regressed? It fails even in Safari 6.0.2.

[for the log: I'm the one who filled the original chromium bug, working on Dojo Toolkit dojox/gfx api).

From my tests, and using the webkit number as it appears in the browser user agent, it looks like the problem appears for webkit version > 534 (AppleWebKit/534.x ). 536 and 537 have the problem, 534 don't.
Comment 5 Philip Rogers 2012-11-29 15:32:06 PST
(In reply to comment #4)
> (In reply to comment #1)
> > Is it known when this regressed? It fails even in Safari 6.0.2.
> 
> [for the log: I'm the one who filled the original chromium bug, working on Dojo Toolkit dojox/gfx api).
> 
> From my tests, and using the webkit number as it appears in the browser user agent, it looks like the problem appears for webkit version > 534 (AppleWebKit/534.x ). 536 and 537 have the problem, 534 don't.

You're right that this is a regression after all.

I looked into this further and did a bisect on the chromium tree. I think the regression range is:
https://trac.webkit.org/log/?verbose=on&stop_rev=105127&rev=105862&limit=10000

There are two likely regressees:
https://bugs.webkit.org/show_bug.cgi?id=76447
https://bugs.webkit.org/show_bug.cgi?id=76446

Assigning this bug to myself.
Comment 6 Philip Rogers 2012-12-01 23:36:18 PST
Created attachment 177132 [details]
Invalidate SVG width on width attribute changes.
Comment 7 Dirk Schulze 2012-12-03 11:22:45 PST
Comment on attachment 177132 [details]
Invalidate SVG width on width attribute changes.

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

> Source/WebCore/svg/SVGSVGElement.cpp:285
> +    bool widthChanged = attrName == SVGNames::widthAttr;

This looks strange. Why isn't it the case for height as well?
Comment 8 Dirk Schulze 2012-12-03 11:37:42 PST
Comment on attachment 177132 [details]
Invalidate SVG width on width attribute changes.

Discussed height on IRC. A test covers height as well. LGTM. r=me.
Comment 9 WebKit Review Bot 2012-12-03 11:45:04 PST
Comment on attachment 177132 [details]
Invalidate SVG width on width attribute changes.

Clearing flags on attachment: 177132

Committed r136424: <http://trac.webkit.org/changeset/136424>
Comment 10 WebKit Review Bot 2012-12-03 11:45:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Philip Rogers 2013-01-15 20:12:42 PST
*** Bug 106923 has been marked as a duplicate of this bug. ***
Comment 12 Dominik Röttsches (drott) 2013-01-16 04:08:40 PST
Philip, could bug 99996 be a duplicate of this as well?
Comment 13 Philip Rogers 2013-01-16 11:12:13 PST
(In reply to comment #12)
> Philip, could bug 99996 be a duplicate of this as well?

I don't think so because this bug is about the root element, whereas 99996 is a drop shadow element. I haven't run the tests to verify this though.
Comment 14 Dirk Schulze 2013-01-16 22:42:00 PST
(In reply to comment #12)
> Philip, could bug 99996 be a duplicate of this as well?

Definitely not. The problem is that FilterFunctions don't have a renderer and don't react correctly on style changes. Everything else is just a hack.