Closed Bug 1706308 Opened 2 months ago Closed 28 days ago

The numbers of imported/not imported logins displayed on the "Import Summary Report" page are not aligned on the same line

Categories

(Firefox :: about:logins, defect, P3)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- unaffected
firefox89 --- disabled
firefox90 --- verified

People

(Reporter: srosu, Assigned: anshukaira)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

[Affected versions]:

  • Firefox Nightly 90.0a1 (Build ID: 20210419221626)

[Affected Platforms]:

  • Mac 11.2.3
  • Windows 10 x64
  • Ubuntu 20.04 x64

[Prerequisites]:

  • Have the latest version of Firefox Nightly installed.
  • Have saved on your computer a CSV file that contains only valid logins.

[Steps to reproduce]:

  1. Navigate to the “about:logins” page.
  2. Click on the Menu button.
  3. Select the “Import from a File…” option.
  4. Select the CSV file from prerequisites and click on the “Open” button from the “Import Logins File” picker.
  5. Click on the “View detailed Import Summary” link.
  6. Observe the numbers of logins from the Summary section.

[Expected result]:

  • The numbers of imported/not imported logins are correctly aligned in the same line.

[Actual result]:

  • The numbers of imported/not imported logins are not correctly aligned.

[Regression]

  • The issue is not reproducible with older Nightly 89.0a1 builds. Considering this using mozregression tools I have found the regression range. Here are the results:
    Last good revision: d64c1382f146846d7c1a1f399f195e3b9c5010d7
    First bad revision: 33850682363d02aece9b45d8d9b7fbc415f6f477
    Pushlog: link
    From this pushlog I think the patch for Bug 1700389 introduced this behavior.

[Notes]:

  • Attached a screenshot of the issue.

hi. i would be taking this bug.
please assign me this bug.
i was asignee of the bug 1700389

Flags: needinfo?(tgiles)
Assignee: nobody → anshusinghjsk
Flags: needinfo?(tgiles)
Priority: -- → P3

:anshukaira, do you have any updates?

Flags: needinfo?(anshusinghjsk)

(In reply to Marco Castelluccio [:marco] from comment #2)

:anshukaira, do you have any updates?

hi. I was a bit busy with my exams. I am done with them by this week though.
I will look into this today/tomorrow.

Flags: needinfo?(anshusinghjsk)

Hi there.
Please take a look at the patch I submitted.
I made :tgiles as my reviewer for now. Please let me know if you are busy or I have to make further changes.
Thanks!!

Flags: needinfo?(tgiles)

Hi Kaira, thanks for letting me know about the patch. I was out of office for a few days, but I'll review this sometime today.

Flags: needinfo?(tgiles)
Attachment #9220856 - Attachment description: Bug 1706308 Fixes alignment of number on Import Summary Page. r=tgiles → Bug 1706308 Fix alignment of numbers on Import Summary Page. r=tgiles
Pushed by tgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d919eb4bfe62
Fix alignment of numbers on Import Summary Page. r=tgiles
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

I have verified this issue using the latest Nightly 90.0a1 (Build ID: 20210512174859) on Windows 10 x64, Ubuntu 20.04 and macOS 11.2.3.

  • It seems that this issue is still reproducible. The numbers of imported/not imported logins are not correctly aligned.

@Tim should I reopen this issue or file a new one?

Flags: needinfo?(tgiles)

Hi Simona, I'll go ahead and reopen this issue. At first this bug looked resolved to me, but I see there's an offset in alignment between the "Existing logins updated" and "Duplicate logins" blocks.

:anshukaira, do you want to add another patch to this or do you want me to take this over? I'm guessing the issue is that there's no container div surrounding the "new-logins", "existing-logins", "duplicate-logins", and "error-logins" divs so grid and flexbox are unsure how to align everyone with respect to one another.

Status: RESOLVED → REOPENED
Flags: needinfo?(tgiles) → needinfo?(anshusinghjsk)
Resolution: FIXED → ---

(In reply to Tim Giles [:tgiles] from comment #10)

:anshukaira, do you want to add another patch to this or do you want me to take this over? I'm guessing the issue is that there's no container div surrounding the "new-logins", "existing-logins", "duplicate-logins", and "error-logins" divs so grid and flexbox are unsure how to align everyone with respect to one another.

Hi :tgiles,
In beginning I thought the problem was with "hidden" keyword and it disturbed the layout.
I am not sure how this behaviour was not there before we toggeled "not-imported".
As the "duplicate-logins" and "error-logins" have one more line which at the end increases their height compared to the other two. And that is the reason for not aligning with each other.
Can we fix their heights, so that all of them are of equal height and hence regardless of "not imported" line they will be aligned.

This is what I think, I might be completely wrong. I can take up on this, but I might ask you a bit of questions. Sorry for that in advance.
Although if you feel that it is taking more time or maybe you should take this over, let me know.

Thanks a lot!

Flags: needinfo?(anshusinghjsk)

:anshukaira, I'm happy to pair program and get this issue resolved, so feel free to reach out with any questions and we'll get it figured out! It sounds like you understand the problem though, we need to determine some way to fix the boxes height issues so that they're aligned across the entire row.

(In reply to Tim Giles [:tgiles] from comment #12)

:anshukaira, I'm happy to pair program and get this issue resolved, so feel free to reach out with any questions and we'll get it figured out! It sounds like you understand the problem though, we need to determine some way to fix the boxes height issues so that they're aligned across the entire row.

Thanks a lot.
So like I was saying we can try fixing the heights of al the boxes so it doesnt matter the "not-imported" line appears or not.
I tried to see if this works, and it seems to be aligning things on my local machine. I am using Ubuntu 18.0.
I tried applying div and giving it a display flex, with align content property. didnt quite work.
So what do you think should be done?? (In reply to Tim Giles [:tgiles] from comment #12)

Flags: needinfo?(tgiles)

Hey Kaira, feel free to upload a WIP patch so I can take a look at it so far and give feedback, thanks!

Flags: needinfo?(tgiles)

Hy, I tried to reopened the already submitted patch.
Umm not sure wht happened, please look at it once. Thanks!!
Even though my changes are listed it doesnt show me needs revision tag.
https://phabricator.services.mozilla.com/D114628
Did I need to open another patch instead of updating??
Thanks

Flags: needinfo?(tgiles)

Hi Kaira, I think in this case what you did is perfectly fine. The new patch appeared in my review queue, so I'll review it shortly!

Flags: needinfo?(tgiles)
Pushed by tgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/04316ebbee1e
Fix alignment of numbers on Import Summary Page. r=tgiles
Status: REOPENED → RESOLVED
Closed: 1 month ago28 days ago
Resolution: --- → FIXED

I have verified this issue using the latest Nightly 90.0a1 (Build ID: 20210518213628) on Windows 10 x64, Ubuntu 20.04 and macOS 11.2.3.

  • The numbers of imported/not imported logins from the “Import Summary Report” page are correctly aligned.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.