162473 - [PATCH] simplify ChangeCSSInlineStyleTxn

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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.

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: 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 ?

Comment 2

[Kathleen :Brade]

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

r=brade

Comment 3

[kinmoz]

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=[e-mail].

Comment 4

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: patch v1.1, carrying forward r=brade and sr=kin, in answer to kin's comments

Comment 5

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

checked in (trunk)