don't include mask-type in property_database.js if the pref is disabled

RESOLVED FIXED in Firefox 20

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

Trunk
mozilla22
x86
macOS
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox20+ fixed, firefox21+ fixed, firefox22 fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments)

Posted patch patchSplinter Review
No description provided.
Attachment #715715 - Flags: review?(bzbarsky)
Attachment #715715 - Flags: approval-mozilla-beta?
Attachment #715715 - Flags: approval-mozilla-aurora?
Comment on attachment 715715 [details] [diff] [review]
patch

r=dbaron... though you should check that test_property_database.html still passes... I'm not sure it will.

The changes to ListCSSProperties in https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/3d9faacd1603/test-webkit-prefixes might be helpful in making it pass if it's not (though they'd need additional modifications).
Attachment #715715 - Flags: review?(bzbarsky) → review+
dbaron is right, this causes test_property_database.html to fail.  Investigating...
Posted patch patch v2Splinter Review
This makes the test pass.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #715777 - Flags: review?(dbaron)
Comment on attachment 715777 [details] [diff] [review]
patch v2

> #define CSS_PROP_ALIAS(name_, id_, method_, pref_) \
>-    { #name_, #method_ },
>+    { #name_, #method_, #pref_ },

If you drop the # here...

>-        printf("}");
>+        if (p->pref && p->pref[0]) {
>+            printf(", pref: ");
>+            if (p->pref[0] == '\"') {
>+                printf("%s", p->pref);
>+            } else {
>+                printf("\"%s\"", p->pref);
>+            }
>+        }
>+        printf(" }");

Then you can drop this wacky check-for-quote code here (and just use the else-half).

I think you can also drop the p->pref null-check; it should always be a string, but it might be empty.

r=dbaron with that, and thanks for jumping on this
Attachment #715777 - Flags: review?(dbaron) → review+
Sorry, I don't know why I landed that before the r+.  Maybe I misread my email and confused with a different bug.  Anyhow, I'll fix the nits on inbound.

https://hg.mozilla.org/releases/mozilla-aurora/rev/b83c82231d13
https://hg.mozilla.org/mozilla-central/rev/f5f3b3a8dc0c
https://hg.mozilla.org/mozilla-central/rev/6d504df6f023
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 715715 [details] [diff] [review]
patch

just cleaning up flags post-beta go to build, these were approved in IRC
Attachment #715715 - Flags: approval-mozilla-beta?
Attachment #715715 - Flags: approval-mozilla-beta+
Attachment #715715 - Flags: approval-mozilla-aurora?
Attachment #715715 - Flags: approval-mozilla-aurora+
Flagging this as [qa-] for verification given in-testsuite coverage. Please remove this whiteboard tag and add the verifyme keyword if this needs QA attention.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.