WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
WIP patch
(2.98 KB, patch)
2011-06-06 22:21 PDT
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Patch
(11.41 KB, patch)
2011-06-07 15:12 PDT
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Patch
(11.39 KB, patch)
2011-06-07 15:25 PDT
,
Simon Fraser (smfr)
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 96306
[details]
Patch
Simon Fraser (smfr)
Comment 7
2011-06-07 15:25:26 PDT
Created
attachment 96312
[details]
Patch
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
http://trac.webkit.org/changeset/88308
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug