1536402 - Private browsing - dotted border in about:downloads section

Status: VERIFIED FIXED

(Toolkit :: Themes, enhancement, P5)

Description

[Cristian Fogel [:cfogel]]

Attachment: privateDLD.png

Affected versions

• 67.0b3, 67.0a1 (2019-03-18), 66.0

Affected platforms

• Windows 10, Ubuntu 16.04, macOS 10.13

Steps to reproduce

1. Launch Firefox in private browsing mode and perform several downloads;
2. Access about:downloads

Expected result

• content is properly displayed;

Actual result

• an interrupted border is displayed (as selection) over the whole white inner section;

Regression range

• Pushlog URL: link
• Last Good build_date: 2015-05-08 changeset: 86203ac87a08
• First Bad build_date: 2015-05-09 changeset: 8f21d63b310a

Additional notes

• attached screenshot with the issue;

Comment 1

[Pierre]

Attachment: Bug 1536402 - fixed botted border on about:downloads in private browsing

Comment 2

[Simon]

Hello, Would it be possible for me to work on this bug? It would be my first!

Comment 3

[Josh Matthews [:jdm]]

Yes! It appears that a previous patch was submitted and there was a review comment that suggested that it wasn't the correct fix, but that should give you a place to start looking for a different fix.

Comment 4

[Simon]

(In reply to Josh Matthews [:jdm] from comment #3)

Yes! It appears that a previous patch was submitted and there was a review comment that suggested that it wasn't the correct fix, but that should give you a place to start looking for a different fix.

Thank you!

Comment 5

[Simon]

Hello, I was wondering if it would be possible for me to gain some insight into the solution I should be considering for this bug. In the previous patch comments it states:

"Maybe we can make it prettier, but I don't think we can just remove the focus ring as this patch does."

What would constitute a prettier solution to this problem? For example, would a solid blue line be both prettier and equally effective in showing focus?

Comment 6

[Josh Matthews [:jdm]]

See comment 5.

Comment 7

[Raghav]

Hi!
Since Simon is already working on this, should I search for something else, or can I also work on it since it's unassigned?

Comment 8

[Josh Matthews [:jdm]]

This issue is small enough that multiple people working on it will mean wasted effort. Please find another bug to look into instead. Thanks for checking first!

Comment 9

[Simon]

Attachment: Bug 1536402 - Changed about:downloads list focus border from dotted to solid.

Changed the focus ring of list of downloads in about:downloads from a dotted blue line to a solid blue line.

Comment 10

[Dão Gottwald [:dao]]

(In reply to Simon from comment #5)

What would constitute a prettier solution to this problem?

I'm not sure, let's ask shorlander. (Tbh I don't understand why this was marked as a good first bug in the first place without some kind of plan for how to address this.)

If we want to change the focus ring, we should do so here: https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/toolkit/themes/shared/in-content/common.inc.css#850

Comment 11

[Simon]

Attachment: Bug 1536402 - Changed about:downloads list focus border from dotted to solid.

Thank you for directing me to the correct file to adjust. I am still not entirely sure whether my current approach of changing the dotted line to a solid line is a correct solution to the problem, but I have moved the change to the suggested file!

Comment 12

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [::dao] from comment #10)

(In reply to Simon from comment #5)

What would constitute a prettier solution to this problem?

I'm not sure, let's ask shorlander. (Tbh I don't understand why this was marked as a good first bug in the first place without some kind of plan for how to address this.)

If we want to change the focus ring, we should do so here: https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/toolkit/themes/shared/in-content/common.inc.css#850

If we are going to change the appearance I would suggest we use the same focus styling we use in about:preferences, about:addons, and about:logins. For example: https://www.dropbox.com/s/q5au64rwiattd9q/Screenshot%202019-09-23%2014.47.48.png?dl=0

Comment 13

[Simon]

Attachment: Bug 1536402 - Changed about:downloads list focus ring to match focus ring of about:preferences, about:addons, and about:logins.

I updated the focus ring it about:downloads to match the other focus rings. Please let me know if I made any mistakes!

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 9095112 [details] [diff] [review]
Bug 1536402 - Changed about:downloads list focus ring to match focus ring of about:preferences, about:addons, and about:logins.

Looks good, thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/879026d0cd13
Let trees and richlistboxes use the same focus ring as input fields. r=dao

Comment 16

[Dão Gottwald [:dao]]

Simon, I pushed your patch manually now since it's your first one, but going forward please submit patches to phabricator so they can be landed from there: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

Comment 17

[Simon]

Okay, Thank you for the heads up!

Comment 18

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/879026d0cd13

Comment 19

[Cristian Fogel [:cfogel]]

Verified with 71.0a1 (2019-10-06) on Windows 10, macOS 10.13, Ubuntu 16.04.

Comment 20

[Cristian Fogel [:cfogel]]

Clearing need-info, amusing it's a miss click.