When you create a "movemail" account (Linux only) and visit the Account settings -> Server settings, the Port field is displayed with a value of -1 and a red outline (indicating error). But this field is not relevant for movemail and should be hidden. The hidefor=movemail attribute is there and a hidden=true attribute gets properly set on the <html:input> element. But it is still visible.

Looks like some problem with <input type="number"> element. Does it properly respond to hidden attribute?

It looks the problem is worse.
Also the subsequent <div> element around username is not hidden for nntp accounts.

Why are there even <div> elements in a xul file? It was done in bug 1546281 for converting away from <grid>. But why not proper <*box> elements? Is <div> even supposed to handle "hidden" attribute?

This simplifies the port elements, but still does not fix the bug.

Uff, so broken in TB 68?

Possibly, you can test the nntp (news) server case on Windows. It shows username field, while it shouldn't.

It seems you guys experiment with 'flex' css here (I don't know why such anomaly) and
the rule #amServerSetting {display: inline-grid; } overrides the global [hidden="true"] rule from the global theme, thus the elements stayed visible.

I add the rule '#amServerSetting [hidden="true"]' but it needs to have 'visibility: hidden', not 'display: none' because otherwise the next element takes its place to fill the third column in the flex grid. With 'visibility' the element takes place, it just isn't visible.

So with this patch the hiding of the elements is restored, but with NNTP the hidden username 'row' is still taking some space so there is this slight ugliness.

But we could take this until someone wants to fight with the flex css more or rewrite it all in <html:table>.

This must go into TB68 fast, as otherwise keeping the input boxes exposed may tempt users to fill them in and then strange things may happen.

I can rewrite this patch with html:table by tomorrow.

Why is a table necessary?

(In reply to Jorg K (GMT+2) from comment #8)

Why is a table necessary?

+1. It is very well known in html-land that <table> has been misused through the ages, and is the reason css grid was invented. Plus, grid lets you easily handle responsive design mode (realigning elements based on viewport width, etc). Although this needs a fast bandaid, we really need to get a strategy.

We have a fast bandaid available. If anybody comes up with something better, then is does not need to be used.
I didn't insist on table. If somebody can fix the flex grid, then great, even though the syntax is a total mess and I won't touch it in the future. XUL grid or HTML table is always easier to grasp.

offtopic: I'm not objecting to a bandaid, but I've seen <table> used elsewhere and that means there's no strategy, which I do object to. I'm pretty sure a pure html/css/js web page jockey could grind out the simple forms that are 90% of the Account Manager UI. XUL grid is dead, it doesn't matter how much we loved it. ;)
btw, +1000 for clientid.

In my opinion this is a tidier way, as it specifies in the XUL how it should be laid out (unfortunately using inline CSS). Also, visibility: hidden leaves a gap below this section for NNTP accounts.

(In reply to alta88 from comment #11)

offtopic: I'm not objecting to a bandaid, but I've seen <table> used
elsewhere and that means there's no strategy, which I do object to.

Looks like one or two unwarranted cases snuck in, but agreed table shouldn't be used for layout.

I modified aceman's patch, so I blame him. ;-) Good points though.

Can we get this landed today, so we can ship it with TB 68 beta 5 which is delayed.

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

+#amServerSetting div:not([hidden="true"]) {

should just be [hidden]

hidden is a boolean attribute, and in would be hidden="hidden" (or not set
at all) in html

The code managing 'hidefor' will add hidden=true as all the elements are supposed to be XUL. Now that we have a mess of html/xul, the things are starting to fall apart. '[hidden]' is fine with me, until there appears an element having hidden=false in the future :)
Is there a good way to query an element whether it is XUL or HTML, e.g. using some namespace? Then we could fix the code to add hidden=hidden where needed.

You can always check Element.namespaceURI but I don't think it's needed. There are not that many boolean attributes, so it's just a matter of checking those.

Aceman, I'll land this now. Maybe you can post a follow-up patch for comment #18.

Yes, land Geoff's version please, which isn't perfect, but the ugliness is only seen with movemail, which is used rarely (compared to nntp).

TB 68 beta 5:
https://hg.mozilla.org/releases/comm-beta/rev/1ced37a21841ea21406900036bdcdb8930f86402

TB 69 beta:
https://hg.mozilla.org/releases/comm-beta/rev/a1430256e067950d4f90b666314b814b1f1b1c52

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/26b1781a4483
Hide unused port settings elements correctly. r=mkmelin

That should have gone into bug 1566281, so I backed it out again :-(
https://hg.mozilla.org/comm-central/rev/6ab99bb2d71a9343480f7f10f2333c67bf4d7ac8