[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 #26 from yichao.yin at torchmobile.com.cn  2009-05-25 20:01 PDT -------
(In reply to comment #24)
> (In reply to comment #23)
> > Created an attachment (id=30576)
 --> (https://bugs.webkit.org/attachment.cgi?id=30576&action=view) [review] [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?

ENABLE(WML)? you mean ENABLE(WCSS)? As for WAP marquee, it is not required by
WML 1.3 but WML 2.0.
And current WebKit just supports WML 1.3. right? 
Like Maciej said, it's not fine to add the property to 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?

Actually, WCSS marquee extension is for XHMLMP. and not WML1.3 but WML2.0 is
required to support it.

> 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.

Yeah, sounds better, I will try to reimplement it. thanks. :)


> 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?

Like I said above, now we needn't consider WML. But you are right, I should 
consider checking if it's an XHTMLMP document to avoid to affect HTML marquee.

> > 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
Thanks for your comments, Niko!
Have a nice day! :)
Yichao


-- 
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.