1326141 - Make adjustHeight method adapt profile item list

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Ray Lin[:ralin]]

Different from the original calculation to the height of autocomplete result, profile item's height might varies according to the length of input box. We should come up a better way to adapt different layout and make its height right.

Comment 1

[Sean Lee [:seanlee][:weilonge]]

bug 1325695 is for handling the Overflow and OverUnderflow cases for each item. That patch might be helpful to handle the case in Form Fill.

Comment 2

[Ray Lin[:ralin]]

Attachment: 2-lines comment in short width

attached this partial spec screenshot to display the issue which might not be handled by current adjustHeight mechanism very well.

Comment 3

[Ray Lin[:ralin]]

(In reply to Sean Lee [:seanlee][:weilonge] from comment #1)
> bug 1325695 is for handling the Overflow and OverUnderflow cases for each
> item. That patch might be helpful to handle the case in Form Fill.

I'll test it out and we shall know if this help then. thank you Sean :)

Comment 4

[Ray Lin[:ralin]]

reopen this bug, since ".collapsed = false" doesn't really hide unused profile-items from height calculation.

Comment 5

[Ray Lin[:ralin]]

quick note:
.collapse for XUL element make computed size return 0. For other elements, "collapse" still affect layout, even the item is rendered fully transparent. (See [0])

[0] https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#Values


----
adding below rule to CSS would solve this issue:

"""
#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-profile"][collapsed="true"] {
  display: none;
}
"""

Comment 6

[Ray Lin[:ralin]]

Attachment: Bug 1326141 - Make .collapse property affect height computation.

Review commit: https://reviewboard.mozilla.org/r/114036/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114036/

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8839355 [details]
Bug 1326141 - Make .collapse property affect height computation.

https://reviewboard.mozilla.org/r/114036/#review115818

::: browser/extensions/formautofill/content/formautofill.css:14
(Diff revision 1)
> +#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-profile"][collapsed="true"] {
> +  display: none;

Ok, and collapse is set in the toolkit autocomplete binding? <richlistitem> is a XUL NS element, right? Perhaps it's actually tied to XUL flexbox and so the problem is `display: block` above should be `display: -moz-box`? Can you try switching to `-moz-box` above instead?

If the above doesn't work can you add a comment like:
// Treat @collpased="true" as `display: none` similar to how it is for XUL elements.
// https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#Values

Comment 8

[Ray Lin[:ralin]]

Comment on attachment 8839355 [details]
Bug 1326141 - Make .collapse property affect height computation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114036/diff/1-2/

Comment 9

[Ray Lin[:ralin]]

Comment on attachment 8839355 [details]
Bug 1326141 - Make .collapse property affect height computation.

https://reviewboard.mozilla.org/r/114036/#review115818

Thanks for your review, I'll land this patch after Bug 1326139.

> Ok, and collapse is set in the toolkit autocomplete binding? <richlistitem> is a XUL NS element, right? Perhaps it's actually tied to XUL flexbox and so the problem is `display: block` above should be `display: -moz-box`? Can you try switching to `-moz-box` above instead?
> 
> If the above doesn't work can you add a comment like:
> // Treat @collpased="true" as `display: none` similar to how it is for XUL elements.
> // https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#Values

I added the comment above this line, since it presents that chaning display back to -moz-box would break HTML flexbox.

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8839355 [details]
Bug 1326141 - Make .collapse property affect height computation.

https://reviewboard.mozilla.org/r/114036/#review115818

> I added the comment above this line, since it presents that chaning display back to -moz-box would break HTML flexbox.

OK, I didn't see this reply because I had already marked r+. I'll autoland this.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bbe2c180655
Make .collapse property affect height computation. r=MattN

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for frequent failure of formautofill/test/chrome/loader_parent.js on Linux x64:

https://hg.mozilla.org/integration/autoland/rev/e2bd855c3e5dc64850b2e31d6902ce64f33abff5

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9bbe2c180655a60ec5e5c94791200c3fca37435e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=79958667&repo=autoland

[task 2017-02-24T13:13:08.274327Z] 13:13:08     INFO - TEST-START | toolkit/components/formautofill/test/chrome/test_infrastructure.html
[task 2017-02-24T13:13:09.434797Z] 13:13:09     INFO - Buffered messages logged at 13:13:09
[task 2017-02-24T13:13:09.435695Z] 13:13:09     INFO - Loading test file: chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/test_infrastructure.js
[task 2017-02-24T13:13:09.436553Z] 13:13:09     INFO - Buffered messages finished
[task 2017-02-24T13:13:09.437658Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Starting loading infrastructure in parent process. - false == true - got false, expected true (operator ==)
[task 2017-02-24T13:13:09.438711Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Registering in the parent process: chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/head_common.js:218 - false == true - got false, expected true (operator ==)
[task 2017-02-24T13:13:09.439754Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Registering in the parent process: chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/head_common.js:244 - false == true - got false, expected true (operator ==)
[task 2017-02-24T13:13:09.444131Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Registering in the parent process: terminationTaskFn - false == true - got false, expected true (operator ==)
[task 2017-02-24T13:13:09.445252Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Finished loading infrastructure in parent process. - false == true - got false, expected true (operator ==)
[task 2017-02-24T13:13:09.449955Z] 13:13:09     INFO - Running wait_loading_in_parent_process
[task 2017-02-24T13:13:09.458693Z] 13:13:09     INFO - Running task in parent process: 
[task 2017-02-24T13:13:09.509655Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Running in the parent process chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/head_common.js:218 - false == true - got false, expected true (operator ==)
[task 2017-02-24T13:13:09.511182Z] 13:13:09     INFO - Finished task in parent process: 
[task 2017-02-24T13:13:09.512161Z] 13:13:09     INFO - Running task in parent process: 
[task 2017-02-24T13:13:09.538938Z] 13:13:09     INFO - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/loader_parent.js | Running in the parent process chrome://mochitests/content/chrome/toolkit/components/formautofill/test/chrome/head_common.js:244 - false == true - got false, expected true (operator ==)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8eea9376ac1
Make .collapse property affect height computation. r=MattN

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Relanded, seems like bug 1341029 is to blame (follow-up pushed).

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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