We have a bunch of stuff left from when prefs were in a window. Now that prefs are in a tab, this causes the the tab to close if you press enter or escape.

Actually we can get rid of the whole binding if we move stuff around a bit.

That would be great.

Richard, please give this a check on all platforms to make sure I haven't missed or broken anything. Especially the two code paths to the dock options on Mac that I can't check.

So this is part of what was planned in bug 1468167?

I guess, but I've never seen that bug before.

Kindly back out https://hg.mozilla.org/comm-central/rev/63b3267ac3ba when you land this with this commit message:
Bug 1523074 - Backed out changeset 63b3267ac3ba to re-enable tests. a=backout

Just opening preferences tab with this patch, I get new errors in console:
ReferenceError: reference to undefined property "value" in display.js:139:5
preference.setElementValue is not a function display.js:117

(In reply to :aceman from comment #8)

Why scripts at the bottom?

The scripts have always been at the bottom due to the way the binding was constructed, but now I want them there so the function in preferences.js can run ASAP. If it waits even until DOMContentLoaded it has to wait for the Lightning overlays which is slooowww. (Probably also the cause of the delay Richard mentioned.)

(In reply to :aceman from comment #10)

Just opening preferences tab with this patch, I get new errors in console:
ReferenceError: reference to undefined property "value" in display.js:139:5
preference.setElementValue is not a function display.js:117

I've seen that but thought it had gone away. I'll look into it more, suspect it may be a timing thing.

We have to wait for preferences.xml to load when adding <preference>s before moving on.

Why does this help with bug 1523074? Different timing now? Plain .js code run at different time than the original XBL bindings?

Probably that, and because we're not adding custom elements (<radio>) to anonymous content.

I still get "ReferenceError: reference to undefined property "value" in display.js:145:5" sometimes.
Try e.g. opening Account manager -> account -> Return receipts -> Global preferences.
Preftab opens in the background but isn't accessible as Account manager is modal. The prefs window in the past could open above it.

Now when you close AM, you get "TypeError: document.getElementById(...) is null in preferences.js:131:7".

That didn't work anyway, but it should be fixed.

I missed changing the comment. :(

(In reply to :aceman from comment #16)

Try e.g. opening Account manager -> account -> Return receipts -> Global preferences.
Preftab opens in the background but isn't accessible as Account manager is modal. The prefs window in the past could open above it.

That's why we should move the Account manager into the prefs tab.

Yes it is a matter of timing.
I still get "ReferenceError: reference to undefined property "value"[Learn More] display.js:145:5" when I have debugging output about what initializes, from radio.xml to the error console.

Really? I thought that would go away when we waited for the earlier binding to load. :(

I'm going to have to cheat to fix that one, because it needs to return synchronously.

Try this, aceman.

Updated patch that applies after bug 1520643's changes of imports in preferences.js.

Thx Richard, forgot about that.

It's on my radar, but since the tree is currently busted, I'm not intending to land this right now.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/1f1658927d2d
Remove prefwindow and preftab bindings. r=aceman

(In reply to Pulsebot from comment #31)

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/1f1658927d2d
Remove prefwindow and preftab bindings. r=aceman

Please backout these changes now as you have just removed shared content with SeaMonkey in common/bindings/preferences.xml

Sorry Ian, common/ is dead, we can't keep suite stuff there just for the point of it. You'll need to move it into suite/ if you need it for something. There is likely little point with that though, as the porting effort is significant, in this case it's not even porting, but close to impossible (see dependent bug).

(In reply to Magnus Melin [:mkmelin] from comment #33)

Sorry Ian, common/ is dead, we can't keep suite stuff there just for the point of it. You'll need to move it into suite/ if you need it for something. There is likely little point with that though, as the porting effort is significant, in this case it's not even porting, but close to impossible (see dependent bug).
It would have been courteous to have at least cc'd some SM developers (which usually does happen).

Sure. I'd be curious to know if there is even an effort to port any of this to SeaMonkey? At current change rates I'd estimate you need at least 2-3 people working full time to have any chance of surviving. At the moment, there are roughly a handful of checkins for SeaMonkey per month. That needs to be the daily rate to even stay near the surface...

Sorry to add insult to injury. SM has received great help from the TB teal in the past. Whenever there is bustage, we try to cover SM as well which in itself is risky business since we can't test.

Factually SM is dead on trunk, as far as I'm aware you haven't even removed overlays which won't load now. I don't even what to talk about XBL. Magnus gave you an estimate of how many developers you need to keep SM running, but he didn't give an estimate of what it would take to work through your huge backlog. Asking for a backout to mitigate item 867 on you backlog list to drop back to 866 is just not a realistic suggestion.

This was a blocker for TB 66 beta which I intend to ship real soon now.

Please don't be me wrong, I very much appreciate the collaboration of FRG, Bill and yourself and whoever else is left from the SM team (now that Neil joined the Owl) effort. We try to help wherever we can, but that doesn't mean that we can help something that already totally dysfunctional.

Yes, we all try to help each other, and sometimes we are busy and forget to include the other. Just sometimes, when you see something big land that removes shared code, one can get a bit overexcited. I would say anything that isn't Firefox does not have an easy life on trunk.

This removed "prefwindow" binding, but we still use that in some prefs subdialogs. See https://dxr.mozilla.org/comm-central/search?q=prefwindow+%2Bpath%3Acomm&redirect=false
Now e.g. their onpaneload handlers do not run (which init the panes) and the dialogs are broken.

:(

Wow, I came to uplift this and I see that it's not finished yet :-( - So what do I do now?

TB 66 beta:
https://hg.mozilla.org/releases/comm-beta/rev/96ed115c265a

I landed what we had, but please provide a patch for the missing work soon.

Open a new bug next time please, aceman. :-/

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/88730121e453
Fix broken loading of preference tab subdialogs. r=aceman,jorgk DONTBUILD

File bug 1526304 for the "Clear Now" failure.

TB 66 beta:
https://hg.mozilla.org/releases/comm-beta/rev/6dbb5631f6aef9ce800b7715dafd332c8a043910

Hmm, sanitise is a stand-alone window, the remaining ones:
https://searchfox.org/comm-central/search?q=%3Cprefwindow&case=false&regexp=false&path=
appear to be subdialogues of the overall pref window. I tried colours and connections and they still work in 66 beta we've just built. Geoff, can you please clarify and file a new bug if necessary.

They're not real dialogs any more, just documents in an iframe. XUL doesn't care what the element is named.

Hmm, maybe room for a clean-up in a new bug ;-)

Then how do the 'onload' and 'ondialogaccept' and 'dlgbuttons' attributes work, if it isn't a dialog?
But we have a ton of css applied to <prefwindow> so migrating would be a lot of churn.

m-c has <dialog class="prefwindow"> in its tests and in https://searchfox.org/comm-central/source/mozilla/browser/components/preferences/colors.xul . But maybe we would then have to convert more to the m-c style of doing things in prefs.

I'm updating a xul addon which has preferences including buttons:
https://github.com/revad/use_bcc_instead_C/blob/master/chrome/content/AddonOptions.xul

The buttons no longer show (67.0a1 2019-02-21). Is that because of this change? If so what to do? One of the buttons displays help text.

(In reply to Dave Royal from comment #54)

I'm updating a xul addon which has preferences including buttons:
https://github.com/revad/use_bcc_instead_C/blob/master/chrome/content/AddonOptions.xul

The buttons no longer show (67.0a1 2019-02-21). Is that because of this change? If so what to do? One of the buttons displays help text.

That is probably because of the inContent options, which now instantApply. That removes the OK and Cancel buttons.

(In reply to Onno Ekker [:nONoNonO UTC+1] from comment #55)

The cancel button is expendable, it's the 'extra' button I'd like to retain.

(In reply to Dave Royal from comment #54)

I'm updating a xul addon which has preferences including buttons:
https://github.com/revad/use_bcc_instead_C/blob/master/chrome/content/AddonOptions.xul

The buttons no longer show (67.0a1 2019-02-21). Is that because of this change? If so what to do? One of the buttons displays help text.

I'm switching our <prefwindow>s to <dialog>s as part of bug 1527770. I'm also moving away from <preferences> and <preference>, so you might want to do the same.