[Webkit-unassigned] [Bug 23727] Implementaion of WCSS marquee extension relevant to XHML MP

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=23727





------- Comment #24 from zimmermann at kde.org  2009-05-23 07:43 PDT -------
(In reply to comment #23)
> Created an attachment (id=30576)
 --> (https://bugs.webkit.org/attachment.cgi?id=30576&action=view) [review]
> Updated patch for code changes
> 
> 
> According to Maciej's comment, fixed the issue of without conditionalization
> constrains for WAP marquee css properties and values.

Patch looks great! Three issues remaining, otherwhise I'd r+ it:

1) Maciej commented before that he'd like a seperated .in file for WCSS before.
Or is it fine to add these properties in ENABLE(WML) blocks to the CSS*.in
files directly?

2) Hardcoded changes in CSSStyleSelector:

> +            if (primitiveValue->getIdent() == CSSValueWapMarquee) {
> +                // Initialize Wap Marquee style 
> +                m_style->setOverflowX(OMARQUEE);
> +                m_style->setOverflowY(OMARQUEE);
> +                m_style->setWhiteSpace(NOWRAP);
> +                m_style->setMarqueeDirection(MLEFT);
> +                m_style->setMarqueeSpeed(85); // Normal speed 
> +                m_style->setMarqueeLoopCount(1);
> +                m_style->setMarqueeBehavior(MSCROLL);

Hm, in WML the marquee mode is enabled using the display="-wap-marquee"
_attribute_ right?
Then you could move this to a WCSS specific .css file, somthing along the
lines:
*[display="-wap-marquee"] { overflow: ... }

The universal selector combined with a specific attribute selector would do.

3) The changes in RenderMarquee concern me a bit, as they'd also affect the
HTML marquee implementation, if WML is enabled. Is that fine? Or shouldn't we
better check if we're operating on a WML document?

> Index: WebCore/rendering/RenderMarquee.cpp
> ===================================================================
> --- WebCore/rendering/RenderMarquee.cpp	(revision 44034)
> +++ WebCore/rendering/RenderMarquee.cpp	(working copy)
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
> + * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
>   *
>   * Portions are Copyright (C) 1998 Netscape Communications Corporation.
>   *
> @@ -196,6 +197,12 @@ void RenderMarquee::updateMarqueePositio
>      if (activate) {
>          EMarqueeBehavior behavior = m_layer->renderer()->style()->marqueeBehavior();
>          m_start = computePosition(direction(), behavior == MALTERNATE);
> +#if ENABLE(WCSS)
> +        // As per the spec WAP-239-WCSS-20011026-a.pdf, if the '-wap-marquee-loop' is '0', no looping occurs.
> +        if (!m_totalLoops)
> +            m_end = m_start;
> +        else 
> +#endif
>          m_end = computePosition(reverseDirection(), behavior == MALTERNATE || behavior == MSLIDE);
>          if (!m_stopped)
>              start();
> @@ -298,6 +305,10 @@ void RenderMarquee::timerFired(Timer<Ren
>          m_currentLoop++;
>          if (m_totalLoops > 0 && m_currentLoop >= m_totalLoops)
>              m_timer.stop();
> +#if ENABLE(WCSS)
> +        else if (!m_totalLoops && m_timer.isActive())
> +            m_timer.stop();
> +#endif
>          else if (s->marqueeBehavior() != MALTERNATE)
>              m_reset = true;
>      }

Have a nice day,
Niko


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.