[Webkit-unassigned] [Bug 6503] content property doesn't support quotes

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=6503





------- Comment #8 from magic at magicninja.org  2008-05-04 11:18 PDT -------
> 1.  There are a bunch of style guideline violations re: placement of "*"
> http://webkit.org/coding/coding-style.html

Fixed.

> I'm confused by this?
> / FIXME: we should use the version present in with CSSPrimitiveValue.cpp
>  37 static String quoteString(const String& string)

Because CSSPrimitiveValue.h does not expose quoteString() for the subclasses
(the function is only declared in CSSPrimitiveValue.cpp). So i had to
copy/paste it.

> (the setTextInternal during a calcPrefsWidths call).  Why is that
> needed/correct?

I fixed this by calling setTextInternal in dirtyLineBoxes. In fact, i don't
know how/when set the internal text; i need the render tree to compute the
depth, so i can't do that in the constructor (since the parent is not yet
known).

> 0 <= m_level (for isVisible) would be more clear as m_level >= 0 IMO.

Fixed.

> Why can't:
> just be
> return m_quotes == o.m_quotes && *m_quotes == *o.m_quotes ?
> yes, that depends on the compiler short-circuting the &&, but that's safe.

I missed the trick :-/ 'm_quotes == o.m_quotes' does not mean '*m_quotes ==
*o.m_quotes' ?

> Bah.  I wish the various setContent calls could share more code.

I'd prefer my patch only corrects the css property. We could open an other bugs
dedicated to this refactoring.

> Any time you're returning an AtomicString, return emptyAtom instead of "";
> there are at least 2 times where you could do that.

Fixed.

> Generally each initializer gets its own line:
>  1068     QuotesData(const QuotesData& o)
>  1069         : m_openQuotes(o.m_openQuotes), m_closeQuotes(o.m_closeQuotes) 

Fixed.

> I think m_quotes is leaking when set.  Can't m_quotes just be an OwnPtr?  I
> don't think it needs to be a DataRef, but that would be another option.

Fixed with OwnPtr.


-- 
Configure bugmail: http://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.