Created attachment 75455 [details] Test case http://trac.webkit.org/changeset/73189 exposed a DOM interface for marquee elements. A marquee element should not animate when its scrollAmount content attribute is set to 0, but setting scrollAmount to 0 in script does not cause the element to stop animating. Instead, it behaves as if scrollAmount was set to 1. See the attached test case.
<rdar://problem/7863203>
Created attachment 75460 [details] Patch
Comment on attachment 75460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75460&action=review A few comments. > WebCore/rendering/RenderMarquee.cpp:297 > + int increment = abs(m_layer->renderer()->style()->marqueeIncrement().calcValue(clientSize)); Why not just change the 1 to a 0? > LayoutTests/fast/html/marquee-scrollamount.html:13 > + // Marquee's default scrollDelay is 85ms, so wait for 100ms to ensure Why no change the scrollDelay to 0 and then wait a shorter time? 100ms doesn't seem long but it adds up over lots of tests.
(In reply to comment #3) > (From update of attachment 75460 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75460&action=review Thanks for the feedback David. > > A few comments. > > > WebCore/rendering/RenderMarquee.cpp:297 > > + int increment = abs(m_layer->renderer()->style()->marqueeIncrement().calcValue(clientSize)); > > Why not just change the 1 to a 0? It seems more concise to remove the call to max() entirely since an absolute value can't possibly be less than 0. > > > LayoutTests/fast/html/marquee-scrollamount.html:13 > > + // Marquee's default scrollDelay is 85ms, so wait for 100ms to ensure > > Why no change the scrollDelay to 0 and then wait a shorter time? > > 100ms doesn't seem long but it adds up over lots of tests. Good idea. I can mke that change before landing.
Committed r73398: <http://trac.webkit.org/changeset/73398>