627594 - SVGLength.value for percent % value lengths doesn't reflect updated viewport until reflow

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: Test case demonstrating problem

See the attached test case.

The <circle>'s cx attribute is defined as 50%, i.e. 50% of the viewport which is initially 100px. If we query the value of the cx attribute in user units we get 50.

After updating the <svg> element's width attribute to 200px and querying the value of the cx attribute we still get 50 user units.

However, after a reflow we get 100 user units.

i.e. Expected output:
<svg> width=100px, 50%=50
<svg> width=200px, 50%=100
force redraw...
<svg> width=200px, 50%=100

Actual output:
<svg> width=100px, 50%=50
<svg> width=200px, 50%=50
force redraw...
<svg> width=200px, 50%=100

The problem is that in this case where we have no viewbox, we are using nsSVGSVGElement's mViewportWidth member to determine the viewport size:
http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGSVGElement.cpp#1130

This member, however, is not updated until we do a reflow:
http://mxr.mozilla.org/mozilla-central/source/layout/svg/base/src/nsSVGOuterSVGFrame.cpp#371

We should update this value immediately.

Interop:
WebKit: as expected (above)
Opera: fails to update value at all

Comment 1

[Jonathan Watt [:jwatt]]

So basically what we have is:

  document.documentElement.setAttribute('width', '...');
  var val = childCircle.cx.baseValue.value;

where the 'cx' attribute is a percentage value. To get that to return the correct value we will either need to reflow synchronously under the setAttribute, or else flush the pending reflow under the .value getter.

Reflowing under the setAttribute doesn't seem to be too desirable since it will in some cases mean we do more reflows than necessary. Flushing reflow when necessary under the .value getter is slightly concerning in that it means we could reflow under a SMIL sample (as pointed out by Brian on IRC). It's definitely bad to flush reflow under a paint, but is in bad under a SMIL sample?

Comment 2

[Robert Longson [:longsonr]]

Attachment: Bug 627594 - flush layout when working with percentage units r=emilio

Comment 3

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=7d562e68576289ba9e869d505e577e7ce14ddc11

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2954a4b8cc4
flush layout when working with percentage units r=emilio

Comment 6

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42400 for changes under testing/web-platform/tests

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/e2954a4b8cc4

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot