storage within CSS declaration blocks (CSSDeclaration) should preserve order of properties

RESOLVED FIXED in mozilla37

Status

()

defect
P2
normal
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: dbaron, Assigned: heycam)

Tracking

(Depends on 1 bug)

Trunk
mozilla37
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We should change our storage of properties within CSS declaration blocks to preserve the order of the declarations (i.e., property:value pairs) within the storage, so that we can use it for things such as bug 649142.
Assignee: nobody → sjohnson
Priority: -- → P2
The work in bug 553456 might or might not take care of this.
Depends on: 553456
Assignee: sjohnson → nobody
Posted patch patchSplinter Review
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8537791 - Flags: review?(dbaron)
Comment on attachment 8537791 [details] [diff] [review]
patch

>+#ifdef DEBUG
>+    // assert that we didn't have any other properties on this expanded data
>+    // block that we didn't find in aOrder
>+    uint32_t numPropsInSet = 0;
>+    for (size_t iHigh = 0; iHigh < nsCSSPropertySet::kChunkCount; iHigh++) {
>+        if (!mPropertiesSet.HasPropertyInChunk(iHigh)) {
>+            continue;
>+        }
>+        for (size_t iLow = 0; iLow < nsCSSPropertySet::kBitsInChunk; iLow++) {
>+            if (mPropertiesSet.HasPropertyAt(iHigh, iLow)) {
>+                numPropsInSet++;
>+            }
>+        }
>+    }
>+    NS_ABORT_IF_FALSE(numPropsNormal + numPropsImportant == numPropsInSet,
>+                      "aOrder missing properties from the expanded data block");
>+#endif

Put a {} scope around the entire contents of the #ifdef DEBUG so the variable inside it doesn't leak outside.

r=dbaron with that

I was hoping that we'd get rid of mOrder entirely, but I guess this is simple and gets us what we need right now.
Attachment #8537791 - Flags: review?(dbaron) → review+
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #3)
> I was hoping that we'd get rid of mOrder entirely, but I guess this is
> simple and gets us what we need right now.

I tried, but having Variables stored on the Declaration was making it difficult.  We can try again as part of bug 553456.
I don't have time to help with any of this at the moment, but I have old patches for bug 553456 at http://hg.mozilla.org/users/zackw_panix.com/patches/ , maybe they are useful at least as a starting point?
https://hg.mozilla.org/mozilla-central/rev/adfa2efc886e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.