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

RESOLVED FIXED in Thunderbird 60.0

Status

defect
RESOLVED FIXED
Last year
Last year

People

(Reporter: jorgk, Assigned: aceman)

Tracking

unspecified
Thunderbird 60.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Thunderbird-testfailure: Z all])

Attachments

(1 attachment)

Reporter

Description

Last year
+++ 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?
Assignee

Comment 1

Last year
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.
Blocks: 1429573
Assignee

Comment 2

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

Comment 3

Last year
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 .
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(paolo.mozmail)
Reporter

Comment 4

Last year
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.
Flags: needinfo?(ntim.bugs)
Assignee

Comment 5

Last year
Posted patch 1437286.patchSplinter Review
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.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #8950007 - Flags: review?(jorgk)
Assignee

Comment 6

Last year
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.
Reporter

Comment 7

Last year
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 ;-)
Flags: needinfo?(richard.marti)
(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.
Flags: needinfo?(ntim.bugs)
Reporter

Comment 9

Last year
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.
Attachment #8950007 - Flags: review?(jorgk) → review+
Reporter

Comment 10

Last year
Oh, maybe we want to change the commit message based on comment #8. Shall we also clear the NI for Alexander and Paolo?
I think what's wrong is, that the 'size' attribute isn't respected. And this is a toolkit issue.
Flags: needinfo?(richard.marti)
Assignee

Comment 12

Last year
(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.
Assignee

Comment 13

Last year
(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.
(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.
I filed bug 1437302 about not respecting the size attribute.
Reporter

Comment 16

Last year
Please NI the relevant developers in bug 1437302.
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(paolo.mozmail)
Assignee

Comment 17

Last year
(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

Last year
Pushed by mozilla@jorgk.com:
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
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Reporter

Updated

Last year
Target Milestone: --- → Thunderbird 60.0
You need to log in before you can comment on or make changes to this bug.