888778 - Custom binding not applied to inline settings

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Geoff Lankow (:darktrojan)]

If an options.xul file contains:

<setting type="mytype"
         style="display: -moz-grid-line; -moz-binding: url('chrome://inlinesettings/content/binding.xml#mytype');"
         title="Custom Setting" />

the binding is not applied. It is applied if the style attribute changes after the page is loaded (e.g. adding a space to the end). Investigating further.

Comment 1

[cricketts]

Are you sure it's that the binding isn't applied? I mean, simply setting "display: -moz-grid-line" in javascript makes it show up as if it were always there to begin with. To me this looks like a CSS engine problem. I was reading another bug report today about working progress on the CSS3 grid line property. Perhaps something that they've been doing with that has affected the original implementation of -moz-grid-line as it was when the <setting> bindings were created? I think that's the most reasonable explaination.

According to https://developer.mozilla.org/en-US/docs/Web/CSS/CSS3 the CSS3 grid propery is in a "Working Draft" status. Whether or not that means it's avaialable in already-released versions, I do not know. But if it is, I'd say it's a likely candidate to have made changes to -moz-grid-line -- perhaps just for inline code or something.

Some clarification on the behavior:

A custom setting does not display and has initial box dimensions of 0x0.

Setting the display property subsequently with JavaScript displays the <setting> as it should

The workaround I made for this was using an observer to observe addon-options-displayed and then set style.display on the element to display it.

I also tried using "display: -moz-grid-line !important" in the XUL to no avail.

Reordering the CSS properties also does not change anything (I got to my wits end with this and tried pretty much everything I could possibly think of).

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: v1

The patch itself adds a call to importNode that should've been there from when I first wrote this. It also removes and reapplies the style attribute, (it seems to do nothing unless it's modified currently, for reasons I can't explain).

The test also includes a simple test for localizing an options.xul, since we should test that, and we don't.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 770133 [details] [diff] [review]
v1

Review of attachment 770133 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay.

Comment 4

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/integration/fx-team/rev/c482e61af458

Comment 5

[Geoff Lankow (:darktrojan)]

Backed out (https://hg.mozilla.org/integration/fx-team/rev/5e3890536f45) because it busted browser_inlinesettings.js. I remember now why there's no importNode call, it breaks an assortment of other settings types. But without it, this patch is broken. Hmm.

Comment 6

[Geoff Lankow (:darktrojan)]

Attachment: 888778-2.diff

This is the same as the previous version, except the importNode call is lower down. This appears to fix both problems.

Comment 7

[Geoff Lankow (:darktrojan)]

Again: https://hg.mozilla.org/integration/fx-team/rev/df15e6a39478

(Try was: https://tbpl.mozilla.org/?tree=Try&rev=60f9bfef1cbb)

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/df15e6a39478