Closed Bug 474739 Opened 11 years ago Closed 10 years ago

SVG SMIL: Timing model does not properly support zero-duration intervals

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: birtles, Assigned: birtles)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch patch v1a (obsolete) — Splinter Review
In converting over my unit tests to mochitests I've come across many scenarios where the SMIL spec seems to be contradictory or ambiguous regarding how to handle zero-duration intervals. I've tested with several implementations and they produce different results. I've posted my queries to www-smil:

http://lists.w3.org/Archives/Public/www-smil/2009JanMar/0002.html
http://lists.w3.org/Archives/Public/www-smil/2009JanMar/0001.html

I've now gone through the specs (SMIL Animation, SMIL 2, SMIL 2 Errata, SMIL3) several times and have arrived at an understanding of what I think the spec intends, namely:

1) begin times CAN be re-used
2) zero-duration intervals ARE allowed
3) non-zero-duration intervals CAN immediately follow zero-duration intervals
4) multiple zero-duration intervals that begin at the same time are skipped (or effectively coalesced into one)

In the attachment I've included many tests for zero-duration intervals which based on this approach. The patch also includes fixes to bring our implementation in to line and also several other fixes to genuine bugs that existed regardless of interpretation but that were revealed by this testing.
Depends on: 216462
(In reply to comment #0)
> Created an attachment (id=358125) [details]
> patch v1a

This patch does not apply cleanly on current trunk.
Attached patch patch v1b (obsolete) — Splinter Review
Fix bitrot.
Attachment #358125 - Attachment is obsolete: true
Attached patch patch v1c (obsolete) — Splinter Review
Fixed a few typos.
Attachment #389654 - Attachment is obsolete: true
Attachment #390956 - Flags: superreview?(roc)
Attachment #390956 - Flags: review?(roc)
Regarding the query to www-smil, the only response received was:

  http://lists.w3.org/Archives/Public/www-smil/2009JanMar/0003.html

which confirms my understanding of the pseudocode (i.e. the erratum from SMIL 2.0 has been mistakenly applied in SMIL 2.0SE, 2.1, and 3) but does not offer any answers about the behaviour of multiple zero-duration intervals, only that, "I think we've simply overlooked the possibility." I think this patch best implements the intention of the spec as described in comment 1 as well as fixing other issues with our implementation that are definitely bugs.
Comment on attachment 390956 [details] [diff] [review]
patch v1c

I just noticed that most of the tests are disabled in that patch.
Attachment #390956 - Flags: superreview?(roc)
Attachment #390956 - Flags: review?(roc)
Attached patch patch v1dSplinter Review
Removed duplicated and unreferenced test code.
Attachment #390956 - Attachment is obsolete: true
Attachment #390965 - Flags: superreview?(roc)
Attachment #390965 - Flags: review?(roc)
Attachment #390965 - Flags: superreview?(roc)
Attachment #390965 - Flags: superreview+
Attachment #390965 - Flags: review?(roc)
Attachment #390965 - Flags: review+
Keywords: checkin-needed
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/d6a573017fba
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.