RESOLVED FIXED 59301
Opacity transition doesn't work while content property is set.
https://bugs.webkit.org/show_bug.cgi?id=59301
Summary Opacity transition doesn't work while content property is set.
shi_a009
Reported 2011-04-24 06:20:34 PDT
The opacity transition doesn't work while content property is set in CSS. Procedure to Reproduce: 1. open the attachment in safari or chrome. 2. wait and see.
Attachments
opacity transition bug (505 bytes, text/html)
2011-04-24 06:22 PDT, shi_a009
no flags
WIP patch (2.98 KB, patch)
2011-06-06 22:21 PDT, Simon Fraser (smfr)
no flags
Patch (11.41 KB, patch)
2011-06-07 15:12 PDT, Simon Fraser (smfr)
no flags
Patch (11.39 KB, patch)
2011-06-07 15:25 PDT, Simon Fraser (smfr)
darin: review+
shi_a009
Comment 1 2011-04-24 06:22:46 PDT
Created attachment 90884 [details] opacity transition bug
Simon Fraser (smfr)
Comment 2 2011-04-25 09:20:23 PDT
We don't support transitions on generated content, but this should still work.
Simon Fraser (smfr)
Comment 3 2011-06-06 21:42:29 PDT
Weird. RenderStyle::setOpacity() is blowing away the m_content of rareNonInheritedData.
Simon Fraser (smfr)
Comment 4 2011-06-06 21:46:29 PDT
StyleRareNonInheritedData's copy ctor totally fails to set up m_content
Simon Fraser (smfr)
Comment 5 2011-06-06 22:21:11 PDT
Created attachment 96203 [details] WIP patch Starter patch, but it would be nice to fix the mess that is ContentData (bug 62185)
Simon Fraser (smfr)
Comment 6 2011-06-07 15:12:29 PDT
Simon Fraser (smfr)
Comment 7 2011-06-07 15:25:26 PDT
Darin Adler
Comment 8 2011-06-07 16:52:29 PDT
Comment on attachment 96312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96312&action=review r=me as is, but I see some opportunity to improve > Source/WebCore/rendering/style/ContentData.cpp:53 > + switch (type()) { Since we already have subclasses for each type of ContentData, it would be better style to use virtual function rather than a switch on type() for the type-specific part of this function. That would eliminate all the calls to static_cast. > Source/WebCore/rendering/style/ContentData.cpp:57 > + RefPtr<StyleImage> image = const_cast<StyleImage*>(static_cast<const ImageContentData*>(this)->image()); There must be some way to avoid this const_cast. Ugh. > Source/WebCore/rendering/style/ContentData.cpp:75 > + if (result && m_next) > + result->setNext(m_next->clone()); It should be straightforward to write this in a loop instead of making it recursive. It’s slightly better to not use stack space proportional to the complexity of the content. > Source/WebCore/rendering/style/CounterDirectives.cpp:45 > + CounterDirectiveMap::const_iterator end = counterDirectives.end(); > + for (CounterDirectiveMap::const_iterator it = counterDirectives.begin(); it != end; ++it) > + result->add(it->first, it->second); You should just be able to say *result = counterDirectives here. Did you try that? > Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp:162 > + if ((!m_counterDirectives && o.m_counterDirectives) || (m_counterDirectives && !o.m_counterDirectives)) > + return false; > + if (m_counterDirectives && o.m_counterDirectives && (*m_counterDirectives != *o.m_counterDirectives)) > + return false; > + return true; > + Your version is OK. My version is the same speed or faster and maybe a little more obvious: if (m_counterDirectives == o.m_counterDirectives) return true; if (m_counterDirectives && o.m_counterDirectives && *m_counterDirectives == *o.m_counterDirectives) return false; return false;
Simon Fraser (smfr)
Comment 9 2011-06-07 18:06:11 PDT
(In reply to comment #8) > Your version is OK. My version is the same speed or faster and maybe a little more obvious: > > if (m_counterDirectives == o.m_counterDirectives) > return true; I had to write this as m_counterDirectives.get() == o.m_counterDirectives.get() > if (m_counterDirectives && o.m_counterDirectives && *m_counterDirectives == *o.m_counterDirectives) > return false; return true there.
Simon Fraser (smfr)
Comment 10 2011-06-07 18:22:54 PDT
Note You need to log in before you can comment on or make changes to this bug.