|Summary:||Setting attr repeatDur=0 on SVG element causes hang|
|Product:||WebKit||Reporter:||Matthew Delaney <mdelaney7>|
|Component:||SVG||Assignee:||Matthew Delaney <mdelaney7>|
|Severity:||Normal||CC:||abarth, bdakin, eric, mdelaney7, webkit.review.bot, zimmermann|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.6|
Description Matthew Delaney 2010-11-01 14:36:55 PDT
There was a hang being caused because the bound checking was just "<0" and not "<=0". As per the spec, a repeatDur of 0 should have no effect: "" repeatDur = "<clock-value>" This causes the element to play repeatedly (loop) for the specified duration. Each repeat iteration lasts for the simple duration. This attribute has no affect if the duration is 0. A repeat duration less than the simple duration specified in dur will cut short the duration. A repeat duration equal to the simple duration is a no-op. Legal values are clock values greater than 0. In addition, "indefinite" may be specified to indicate that the element should repeat indefinitely (subject to the time container semantics). At most one of repeat or repeatDur should be specified (if both are specified, the repeat duration is defined as the minimum of the specified repeatDur, and the simple duration multiplied by the repeat (count). ""
Comment 2 Simon Fraser (smfr) 2010-11-01 14:59:36 PDT
Comment on attachment 72563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72563&action=review > WebCore/svg/animation/SVGSMILElement.cpp:531 > - return m_cachedRepeatDur = clockValue < 0 ? SMILTime::unresolved() : clockValue; > + return m_cachedRepeatDur = clockValue <= 0 ? SMILTime::unresolved() : clockValue; The assignment in the return statement is pretty grody here. Please change to: m_cachedRepeatDur = clockValue <= 0 ? SMILTime::unresolved() : clockValue; return m_cachedRepeatDur; > LayoutTests/svg/animations/repeatDur-zero.xhtml:10 > +</svg> > \ No newline at end of file Please add the newline.
Comment 4 Matthew Delaney 2010-11-01 15:41:03 PDT
Committed r71066: <http://trac.webkit.org/changeset/71066>
Comment 5 WebKit Review Bot 2010-11-01 16:05:05 PDT
http://trac.webkit.org/changeset/71066 might have broken GTK Linux 64-bit Debug The following tests are not passing: accessibility/aria-activedescendant-crash.html accessibility/aria-checkbox-text.html accessibility/aria-hidden-update.html accessibility/contenteditable-hidden-div.html accessibility/crash-with-noelement-selectbox.html accessibility/crashing-a-tag-in-map.html accessibility/document-attributes.html accessibility/first-letter-text-transform-causes-crash.html accessibility/hang-in-isignored.html accessibility/nested-layout-crash.html accessibility/nochildren-elements.html accessibility/non-data-table-cell-title-ui-element.html accessibility/non-native-image-crash.html accessibility/radio-button-checkbox-size.html accessibility/removed-anonymous-block-child-causes-crash.html accessibility/removed-continuation-element-causes-crash.html accessibility/table-modification-crash.html accessibility/table-nofirstbody.html accessibility/table-notbody.html accessibility/table-with-empty-thead-causes-crash.html
Comment 6 Nikolas Zimmermann 2010-11-03 08:38:58 PDT
Matthew, can you change the testcase to include if (window.layoutTestController) layoutTestController.dumpAsText() there's no need to dump render tree information there. At the moment the expected.png/checksum files are missing, but it's useless to add them anyway. When doing that, you might also want to remove the existing expected.txt files from platform/mac/svg/animations, and from the gtk/qt/win/chromium* platform directories (in case they have already landed their own results), and move the expected.txt file right into svg/animations/ Thanks in advance!