Patch to follow.
Created attachment 125652 [details] Patch
Comment on attachment 125652 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125652&action=review > Source/WebCore/ChangeLog:5 > + Could you provide a high level overview of the undo approach? > Source/WebCore/inspector/InspectorCSSAgent.cpp:108 > + if (!inspectorStyleSheet->text(&m_oldText)) text -> getText? > Source/WebCore/inspector/InspectorCSSAgent.cpp:161 > + return "SetPropertyText" + m_styleSheetId + ":" + String::number(m_propertyIndex) + ":" + (m_overwrite ? "true" : "false"); Use String::format? > Source/WebCore/inspector/InspectorCSSAgent.cpp:169 > + m_text = other->m_text; This is not only needed for toString, right? > Source/WebCore/inspector/InspectorCSSAgent.h:101 > + friend class SetStyleSheetTextAction; You can make these classes nested and just forward declare them here. > Source/WebCore/inspector/InspectorDOMAgent.cpp:135 > +class RemoveChildAction : public DOMAction { All actions should be marked as WTF_MAKE_NONCOPYABLE > Source/WebCore/inspector/InspectorDOMAgent.cpp:290 > +private: style: blank like above private: > Source/WebCore/inspector/InspectorHistory.cpp:106 > + while (!m_history.isEmpty() && m_history.first()->isUndoableStateMark()) Instead of marking end of action, you could mark its beginning which would be simpler. > Source/WebCore/inspector/InspectorHistory.cpp:107 > + m_history.takeFirst(); removeFirst > Source/WebCore/inspector/InspectorHistory.h:50 > +class InspectorHistory { WTF_MAKE_NONCOPYABLE > Source/WebCore/inspector/InspectorStyleSheet.h:177 > + bool setPropertyText(ErrorString*, const InspectorCSSId&, unsigned propertyIndex, const String& text, bool overwrite, String* oldPropertyText); oldPropertyText -> oldText as in other places
(In reply to comment #2) > (From update of attachment 125652 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125652&action=review > > > Source/WebCore/ChangeLog:5 > > + > > Could you provide a high level overview of the undo approach? Done. > > > Source/WebCore/inspector/InspectorCSSAgent.cpp:108 > > + if (!inspectorStyleSheet->text(&m_oldText)) > > text -> getText? > Done. > > Source/WebCore/inspector/InspectorCSSAgent.cpp:161 > > + return "SetPropertyText" + m_styleSheetId + ":" + String::number(m_propertyIndex) + ":" + (m_overwrite ? "true" : "false"); > > Use String::format? > Done. > > Source/WebCore/inspector/InspectorCSSAgent.cpp:169 > > + m_text = other->m_text; > > This is not only needed for toString, right? > This will also be used in redo. > > Source/WebCore/inspector/InspectorCSSAgent.h:101 > > + friend class SetStyleSheetTextAction; > > You can make these classes nested and just forward declare them here. Done. > > > Source/WebCore/inspector/InspectorDOMAgent.cpp:135 > > +class RemoveChildAction : public DOMAction { > > All actions should be marked as WTF_MAKE_NONCOPYABLE Done. > > > Source/WebCore/inspector/InspectorDOMAgent.cpp:290 > > +private: > > style: blank like above private: > Done. > > Source/WebCore/inspector/InspectorHistory.cpp:106 > > + while (!m_history.isEmpty() && m_history.first()->isUndoableStateMark()) > > Instead of marking end of action, you could mark its beginning which would be simpler. This would make the code harder due to minor (non-major) style edits. > > > Source/WebCore/inspector/InspectorHistory.cpp:107 > > + m_history.takeFirst(); > > removeFirst Done. > > > Source/WebCore/inspector/InspectorHistory.h:50 > > +class InspectorHistory { > > WTF_MAKE_NONCOPYABLE > Done. > > Source/WebCore/inspector/InspectorStyleSheet.h:177 > > + bool setPropertyText(ErrorString*, const InspectorCSSId&, unsigned propertyIndex, const String& text, bool overwrite, String* oldPropertyText); > > oldPropertyText -> oldText as in other places In other places this method is defined on style, not stylesheet.
Created attachment 125819 [details] [Patch] for landing
Created attachment 125829 [details] [Patch] with tests.
Committed r106932: <http://trac.webkit.org/changeset/106932>
Hi. I think there's a chance this broke mac build see in http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Build%29/builds/38577/steps/compile-webkit/logs/stdio (look for "OwnPtr" in the file)
Reopen, because the build is still broken.
Committed r106953: <http://trac.webkit.org/changeset/106953>