Closed
Bug 1024860
Opened 7 years ago
Closed 7 years ago
Empty SVG Path getTotalLength() throws exception in JS
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: alancutter, Assigned: longsonr)
Details
Attachments
(1 file)
4.03 KB,
patch
|
jwatt
:
review+
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.49 Safari/537.36 Steps to reproduce: Call getTotalLength() on an SVG Path element with an empty data field. http://jsfiddle.net/PGPqM/12/ Actual results: [Exception... "Failure" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: http://fiddle.jshell.net/PGPqM/12/show/ :: window.onload :: line 23" data: no] Expected results: Total path length: 0 This test passes.
Assignee | ||
Updated•7 years ago
|
Component: Untriaged → SVG
Product: Firefox → Core
Assignee | ||
Comment 1•7 years ago
|
||
Assignee: nobody → longsonr
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #8441186 -
Flags: review?(jwatt)
Assignee | ||
Comment 2•7 years ago
|
||
I'll adapt the mochitest from bug 1024926 to cover this case before landing i.e. start from a d="" measure and check that and then set the path in the bug 1024926 mochitest using setAttribute.
Comment 3•7 years ago
|
||
Comment on attachment 8441186 [details] [diff] [review] length.txt Review of attachment 8441186 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/svg/content/src/SVGPathElement.cpp @@ +347,5 @@ > float authorsPathLengthEstimate = mPathLength.GetAnimValue(); > if (authorsPathLengthEstimate > 0) { > RefPtr<Path> path = GetPathForLengthOrPositionMeasuring(); > + if (!path) { > + return 0.0; Return 1.0 as we do at the end of this function. And remove the trailing whitespace from this line.
Attachment #8441186 -
Flags: review?(jwatt) → review+
Assignee | ||
Comment 4•7 years ago
|
||
> Return 1.0 as we do at the end of this function. And remove the trailing
> whitespace from this line.
Returning 1.0 would be wrong at that point. RefPtr<Path> path = GetPathForLengthOrPositionMeasuring(); returns null if the path is empty i.e. its a zero length path.
We want to return path->ComputeLength() / authorsPathLengthEstimate; and we've tested that authorsPathLengthEstimate > 0 so in this inner bit of code that would come out as 0 (it used to do that before moz2d came along and made empty paths return nullptr)
You want some comments to help future readers?
Flags: needinfo?(jwatt)
Assignee | ||
Comment 6•7 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=bd677df9cfe2
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8441186 [details] [diff] [review] length.txt Apparently the webidl change needs a DOM peer review
Attachment #8441186 -
Flags: review?(bzbarsky)
![]() |
||
Comment 8•7 years ago
|
||
Comment on attachment 8441186 [details] [diff] [review] length.txt r=me
Attachment #8441186 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 9•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/82210155467c
Flags: in-testsuite+
Comment 10•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/82210155467c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in
before you can comment on or make changes to this bug.
Description
•