[PATCH] simplify ChangeCSSInlineStyleTxn

RESOLVED FIXED

Status

()

Core
Editor
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: glazou, Assigned: glazou)

Tracking

({memory-footprint})

Trunk
memory-footprint
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
Bug 60683 has been fixed recently. This bug prevented from assigning inline CSS
styles to an element outside of document's tree.

Rewriting ChangeCSSInlineStyleTxn to take advantage of that fix - see for
instance the URL attached above - should (a) simplify a lot the Do() part of
the transaction (b) reduce the size of the strings kept in the txn.
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Depends on: 60683
Keywords: footprint
(Assignee)

Comment 1

16 years ago
Created attachment 95095 [details] [diff] [review]
patch v1.0

This patch makes a ChangeCSSInlineStyleTxn keep track only of the values of the

requested CSS property before and after txn, instead of keeping track of the
values of the whole STYLE attribute.
It means that we save A LOT of space. Example : we have a <span
style="color:red">
and want to apply to it a blue background. Instead of keeping in the txn the
strings

    "color: red;"
and "color: red; background-color: blue; "

we now only keep "" and "blue" :-)

Thanks to bz who made that possible fixing bug 60683.

Kathy, Kin, r/sr please ?
(Assignee)

Updated

16 years ago
Summary: simplify ChangeCSSInlineStyleTxn → [PATCH] simplify ChangeCSSInlineStyleTxn
Whiteboard: fix in hand, needs r=, needs sr=

Comment 2

16 years ago
Comment on attachment 95095 [details] [diff] [review]
patch v1.0

r=brade
Attachment #95095 - Flags: review+

Comment 3

16 years ago
Comment on attachment 95095 [details] [diff] [review]
patch v1.0

-- Are you assigning into |result| for debugging purposes?


+  result = cssDecl->GetPropertyValue(propertyNameString, mRedoValue);
   return result;


-- In your change to UndoTransaction(), you are checking to see if
   |mRedoValue.IsEmpty()|, should that be |mUndoValue.IsEmpty()|?


-- Also the bulk of the code you add in both UndoTransaction() and
   RedoTransaction() looks the same. Should it be in a utility method?


Address the points above and you got an sr=kin@netscape.com.
Attachment #95095 - Flags: superreview+
(Assignee)

Comment 4

16 years ago
Created attachment 96002 [details] [diff] [review]
patch v1.1, carrying forward r=brade and sr=kin, in answer to kin's comments
Attachment #95095 - Attachment is obsolete: true
(Assignee)

Comment 5

16 years ago
checked in (trunk)
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Whiteboard: fix in hand, needs r=, needs sr=
You need to log in before you can comment on or make changes to this bug.