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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 2

[Cameron McCormack (:heycam)]

dbaron is right, this causes test_property_database.html to fail.  Investigating...

Comment 3

[Cameron McCormack (:heycam)]

Attachment: patch v2

This makes the test pass.

Comment 4

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/releases/mozilla-beta/rev/00409be5072b

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 6

[Cameron McCormack (:heycam)]

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

Comment 7

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5f3b3a8dc0c

Comment 8

[Cameron McCormack (:heycam)]

Address review comments:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d504df6f023

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f5f3b3a8dc0c
https://hg.mozilla.org/mozilla-central/rev/6d504df6f023

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 715715 [details] [diff] [review]
patch

just cleaning up flags post-beta go to build, these were approved in IRC

Comment 11

[u279076]

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.