domUtils.getCSSValuesForProperty fails on: - mix-blend-mode - text-combine-horizontal - text-orientation - writing-mode. Here is a simple scratchpad that demonstrates the problem: let DOMUtils = Cc["@mozilla.org/inspector/dom-utils;1"] .getService(Ci.inIDOMUtils); let target = content.document.body; let names = content.window.getComputedStyle(target); for (let name of names) { try { DOMUtils.getCSSValuesForProperty(name); } catch(e) { dump("Cannot get values for " + name + "\n"); } }

These properties are all disabled by default (as in, do not exist, as far as Gecko is concerned). Did you test in a build in which you enabled them?

I tested on fx-team so I guess so.

Why would they be enabled on fx-team, exactly?

I don't know. All I know is that using: content.window.getComputedStyle(content.document.body); returns properties including mix-blend-mode, text-combine-horizontal, text-orientation & writing-mode on fx-team

"returns" in what sense? What actual code are you using here? Certainly |"mixBlendMode" in content.window.getComputedStyle(content.document.body)| should be returning false...

So run the following in scratchpad: let target = content.document.body; let properties = content.window.getComputedStyle(target); for(let name of properties) { if (name === "mix-blend-mode" || name === "text-combine-horizontal" || name === "text-orientation" || name === "writing-mode") { dump("name: '" + name + "'\n"); } } ////////////////// The problem is that in order to get a full list of properties for the Computed CSS View we use getComputedStyle() to get all available property names (they are all displayed when browser styles is checked). Iterating over the CSS2Properties object using a for of loop iterates through mix-blend-mode etc. and because the value is undefined we are left with blank lines in the computed view. If these properties are disabled then can they be removed from the CSS2Properties object?

> for(let name of properties) { Ah, the indexed props! Those are just buggy, looks like.

Though fixing this while keeping the prefs live during a brower session but not regressing performance is actually pretty darned hard. David, how ok would you be with nsComputedDOMStyle::GetQueryablePropertyMap reflecting the pref state of the very first time it's called?

Hmmm. Maybe bug 264517 will get around the problem? Or does the issue have something to do with the way the bindings code constructs a prototype? I guess if this is per-element or per-page it's probably ok, though.

The issue is that nsComputedDOMStyle code has a big static array of property names. When you ask it for its .length, it returns the length of that array. When you ask it for [n] for some integer n it indexes into that array. The "right" way to handle preferences, in theory, would be to store the enabled state in the array too. Then .length would walk the array, counting the enabled props. And [n] would walk the array looking for the n-th enabled prop. But that of course makes .length O(N) and iteration O(N^2). The cheap option is to construct the array once, then go through memmoving things in it to get rid of disabled stuff. But that's not live if prefs change during runtime. This is basically what I was proposing in comment 8. I guess we could stop using a static array and hang an array off the document. Or even off the computed style object itself, I suppose... That feels dangerous in terms of memory use and iteration performance if people keep getting new computed style objects, though.

Is the order that the properties are exposed important? You could swap disabled ones to the end of the array, rather than shifting everything across, if not. Another option would be to have a static array that maps from indexes-excluding-disabled-properties to indexes-including-disabled-properties which gets updated when a pref changes.

> Is the order that the properties are exposed important? Technically, it'll be standardized at some point. And I think people kinda expect them in alphabetical order. > Another option would be to have a static array that maps from > indexes-excluding-disabled-properties to indexes-including-disabled-properties which > gets updated when a pref changes. Ah, yes. I like this. This shouldn't be too bad to set up. So cache the length and the index-to-index mapping, then update the cache in the unlikely case that the pref changes. Want to do it? ;)

Sure, I'll take a look.

Comment on attachment 812530 [details] [diff] [review] Part 1: Move computed style map entries to a preprocessor included file. r=me

Comment on attachment 812531 [details] [diff] [review] Part 2: Encapsulate the computed style map and make it take disabled properties into account. Beautiful. r=me Add a test?

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/67d83e429b29 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8f5e68fc1f6 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/512d5d35712e

https://hg.mozilla.org/mozilla-central/rev/67d83e429b29 https://hg.mozilla.org/mozilla-central/rev/a8f5e68fc1f6 https://hg.mozilla.org/mozilla-central/rev/512d5d35712e