1437286 - TEST-UNEXPECTED-FAIL | [snip]/mozmill/account/test-account-port-setting.js | test-account-port-setting.js::test_account_port_setting

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1437269 +++

This failed before due to event issues in bug 1437269. With M-C at rev c2cddb0cbb20 and the patch from bug 1437269 applied, the test passes.

So another bustage between c2cddb0cbb20 and currently a8e153c55eeee93a11e87d325fb20c6444.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c2cddb0cbb20&tochange=a8e153c55eeee93a11e87d325fb20c6444

https://public-artifacts.taskcluster.net/aHtAl1p5QY6fH9ppMBNqSQ/0/public/logs/live_backing.log
Failure says:
INFO -  SUMMARY-UNEXPECTED-FAIL | test-account-port-setting.js | test-account-port-setting.js::test_account_port_setting
INFO -    EXCEPTION: An attempt was made to use an object that is not, or is no longer, usable
INFO -      at: nonesuch line 124
INFO -         set_selectionEnd textbox.xml:124 1
INFO -         subtest_check_set_port_number test-account-port-setting.js:41 3

Code is:
   let iframe = amc.window.document.getElementById("contentFrame");
   let portElem = iframe.contentDocument.getElementById("server.port");
   portElem.focus();
41 portElem.selectionStart = portElem.selectionEnd = portElem.value.length;

That line 41 looks weird to me. What is it trying to achieve?

Comment 1

[:aceman]

They must have changed something in <textbox type="number"> handling. It can be seen visually in account settings/server that all numeric textboxes are now too long and the arrows are ugly spaced (at least in Linux).
Possibly broken by bug 1429573.

Comment 2

[:aceman]

It looks like 'size' attribute on the textbox is not honored. But I do not see any change in this area in bug 1429573.

Comment 3

[:aceman]

I could fix the test os it passes as it does not really need to access .selectionStart and .selectionEnd. But the question still remains why would textbox.selectionStart throw that error. It is still used all over the place in m-c and c-c.
The code in question is at https://dxr.mozilla.org/comm-central/rev/1a8f56b5d40991354b30c019cfa805e970fb2673/mail/test/mozmill/account/test-account-port-setting.js#41 .

Comment 4

[Jorg K (CEST = GMT+2)]

Aceman, can you please attach a patch to make the test pass. I don't see any value-add for us to have a test fail that tests port numbers in the account manager due to some number field issue.

Also NI'ing the patch author from bug 1429573.

Comment 5

[:aceman]

Attachment: 1437286.patch

Yes, this should work, clears the field using a different method.

But we must not forget the broken textbox appearance, maybe some css or xul needs to be adopted into c-c.

Comment 6

[:aceman]

Comment on attachment 8950007 [details] [diff] [review]
1437286.patch

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

The bug number in the commit message is wrong.

Comment 7

[Jorg K (CEST = GMT+2)]

OK, thanks, I'll look at it soon.

Meanwhile, Richard, is there something wrong with the textbox appearance due to bug 1429573. Could you please check in a local build? And if so, file a bug ... and fix it ;-)

Comment 8

[Tim Nguyen :ntim]

(In reply to :aceman from comment #3)
> I could fix the test os it passes as it does not really need to access
> .selectionStart and .selectionEnd. But the question still remains why would
> textbox.selectionStart throw that error. It is still used all over the place
> in m-c and c-c.
> The code in question is at
> https://dxr.mozilla.org/comm-central/rev/
> 1a8f56b5d40991354b30c019cfa805e970fb2673/mail/test/mozmill/account/test-
> account-port-setting.js#41 .

According to https://html.spec.whatwg.org/#do-not-apply, input[type=number] has no concept of selectionStart/selectionEnd.

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8950007 [details] [diff] [review]
1437286.patch

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

I'll test it when my build finishes. r+ assuming that it works ;-)

::: mail/test/mozmill/account/test-account-port-setting.js
@@ +46,5 @@
>  
>      mc.sleep(0);
>    }
>  
> +  mc.click(new elib.Elem(amc.window.document.getElementById("accountManager").getButton("accept")));

I can see a bit of boy-scout clean-up in the other hunks. What is this one about? Nit: The line is also very long.

I can fix that together with the bug number before landing.

Comment 10

[Jorg K (CEST = GMT+2)]

Oh, maybe we want to change the commit message based on comment #8. Shall we also clear the NI for Alexander and Paolo?

Comment 11

[Richard Marti (:Paenglab)]

I think what's wrong is, that the 'size' attribute isn't respected. And this is a toolkit issue.

Comment 12

[:aceman]

(In reply to Tim Nguyen :ntim from comment #8)
> According to https://html.spec.whatwg.org/#do-not-apply, input[type=number]
> has no concept of selectionStart/selectionEnd.

Thanks, then maybe https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/textbox should be updated.

(In reply to Jorg K (GMT+1) from comment #9)
> > +  mc.click(new elib.Elem(amc.window.document.getElementById("accountManager").getButton("accept")));
> I can see a bit of boy-scout clean-up in the other hunks. What is this one
> about? Nit: The line is also very long.

Better simulating user interaction (clicking the button) instead of calling internal methods. mozmill tests should model what the user can do, where possible.

Comment 13

[:aceman]

(In reply to Richard Marti (:Paenglab) from comment #11)
> I think what's wrong is, that the 'size' attribute isn't respected. And this
> is a toolkit issue.

Yes, but whether you can spot the change which caused it.

Comment 14

[Richard Marti (:Paenglab)]

(In reply to :aceman from comment #13)
> (In reply to Richard Marti (:Paenglab) from comment #11)
> > I think what's wrong is, that the 'size' attribute isn't respected. And this
> > is a toolkit issue.
> 
> Yes, but whether you can spot the change which caused it.

The last TB tinderbox before bug 1429573 was okay field-size wise. I see only bug 1429573 touching this field.

Comment 15

[Richard Marti (:Paenglab)]

I filed bug 1437302 about not respecting the size attribute.

Comment 16

[Jorg K (CEST = GMT+2)]

Please NI the relevant developers in bug 1437302.

Comment 17

[:aceman]

(In reply to Tim Nguyen :ntim from comment #8)
> According to https://html.spec.whatwg.org/#do-not-apply, input[type=number]
> has no concept of selectionStart/selectionEnd.

In that same table 'size' is also not supported :(
So it seems input[type=number] can't really implement textbox[type=number].

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6d9e121245b3
don't use .selectionStart on a 'textbox type="number"' in test-account-port-setting.js since it's not supported after bug 1429573. r=jorgk