Open
Bug 1383776
Opened 7 years ago
Updated 2 years ago
Identity icon isn't horizontally centered on hover
Categories
(Firefox :: Theme, defect, P3)
Tracking
()
NEW
People
(Reporter: afnankhan, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170724030204
Updated•7 years ago
|
Component: Untriaged → Site Identity and Permission Panels
Updated•7 years ago
|
Component: Site Identity and Permission Panels → Theme
Comment 1•7 years ago
|
||
Yeah I've been annoyed by that as well already but didn't get around to filing it. Thanks for reporting the bug. :)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [photon-visual][triage]
Updated•7 years ago
|
Flags: qe-verify+
Priority: -- → P3
QA Contact: brindusa.tot
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
Updated•7 years ago
|
Priority: P3 → P4
Updated•7 years ago
|
Priority: P4 → P3
Updated•7 years ago
|
Assignee: nobody → jhofmann
Blocks: photon-visual
Status: NEW → ASSIGNED
status-firefox56:
--- → unaffected
status-firefox57:
--- → affected
status-firefox58:
--- → affected
Priority: P3 → P1
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
This patch doesn't perfectly center it (it's not possible without making the identity block larger or having no space between the hover background and the url text), but it improves things to the point where I'm personally ok with it.
Dale, what do you think?
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8913612 [details]
Bug 1383776 - Shift identity box hover state slightly to the right when there are no security indicators.
https://reviewboard.mozilla.org/r/185000/#review190080
::: browser/themes/shared/identity-block/identity-block.inc.css:21
(Diff revision 1)
> overflow: hidden;
> }
>
> +#identity-box.unknownIdentity {
> + /* Give a little more space to the right when
> + * there's a single icon in the identity block, to
#identity-box.unknownIdentity != "single icon in the identity block"
E.g. there's the insecure login forms icon, tracking protection, permission and notification icons...
Comment 5•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #4)
> Comment on attachment 8913612 [details]
> Bug 1383776 - Shift identity box hover state slightly to the right in
> unknown identity mode.
>
> https://reviewboard.mozilla.org/r/185000/#review190080
>
> ::: browser/themes/shared/identity-block/identity-block.inc.css:21
> (Diff revision 1)
> > overflow: hidden;
> > }
> >
> > +#identity-box.unknownIdentity {
> > + /* Give a little more space to the right when
> > + * there's a single icon in the identity block, to
>
> #identity-box.unknownIdentity != "single icon in the identity block"
>
> E.g. there's the insecure login forms icon, tracking protection, permission
> and notification icons...
Ah, yes, you're right, for some reason I thought the classnames get replaced, e.g. insecure login forms only has the insecureLoginForms classname, not unknownIdentity.
Updated•7 years ago
|
Attachment #8913612 -
Flags: review?(dharvey)
Comment 6•7 years ago
|
||
In retrospective I'm not really sure what made me think this patch was gonna work, there are also the permission icons (which I added myself) to consider. I'm gonna put up a patch that I'm not really proud of just to make sure I've exhausted all the options available here.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8913612 -
Flags: review?(dharvey)
Comment 8•7 years ago
|
||
So this patch has two flaws:
- It's horrible and breaks once anyone changes things in the identity block and doesn't pay attention. (It's hard to test on top of that)
- It doesn't cover anchor icons for permission prompts (because that's a bit harder I think), which is an edge case I could live with.
I'm not putting it up for review because I'm not really sure I want to go through with it but I'm also kind of out of ideas on how to solve this best.
I'll un-assign myself and put this back into the backlog (with low prio). If we think this is indeed a bigger problem I'm happy to push this patch through (or a better patch if anyone has a great idea).
Assignee: jhofmann → nobody
Status: ASSIGNED → NEW
Priority: P1 → P4
Updated•7 years ago
|
Flags: qe-verify+
Priority: P4 → P3
QA Contact: brindusa.tot
Whiteboard: [reserve-photon-visual]
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•