Make adjustHeight method adapt profile item list

RESOLVED FIXED in Firefox 54

Status

()

Toolkit
Form Manager
RESOLVED FIXED
6 months ago
4 months ago

People

(Reporter: ralin, Assigned: ralin)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [form autofill:M1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

6 months ago
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.
See Also: → bug 1325695
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.
(Assignee)

Comment 2

6 months ago
Created attachment 8823492 [details]
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.
(Assignee)

Comment 3

6 months ago
(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 :)
(Assignee)

Updated

5 months ago
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → INVALID
(Assignee)

Comment 4

4 months ago
reopen this bug, since ".collapsed = false" doesn't really hide unused profile-items from height calculation.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(Assignee)

Comment 5

4 months ago
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 hidden (mozreview-request)
(Assignee)

Updated

4 months ago
Assignee: nobody → ralin
Status: REOPENED → ASSIGNED
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
Attachment #8839355 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 9

4 months ago
mozreview-review-reply
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 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

4 months ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/9bbe2c180655
Make .collapse property affect height computation. r=MattN
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 ==)
Flags: needinfo?(MattN+bmo)

Comment 13

4 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/d8eea9376ac1
Make .collapse property affect height computation. r=MattN
Relanded, seems like bug 1341029 is to blame (follow-up pushed).
Flags: needinfo?(MattN+bmo)

Comment 15

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d8eea9376ac1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago4 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.