Open Bug 1383776 Opened 3 years ago Updated 1 year ago

Identity icon isn't horizontally centered on hover

Categories

(Firefox :: Theme, defect, P3)

56 Branch
defect

Tracking

()

Tracking Status
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox66 --- wontfix
firefox67 --- affected
firefox68 --- affected

People

(Reporter: afnankhan, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached image Identity icon
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170724030204
Component: Untriaged → Site Identity and Permission Panels
Component: Site Identity and Permission Panels → Theme
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]
Flags: qe-verify+
Priority: -- → P3
QA Contact: brindusa.tot
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
Priority: P3 → P4
Priority: P4 → P3
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: P3 → P1
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 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...
(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.
Attachment #8913612 - Flags: review?(dharvey)
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.
Attachment #8913612 - Flags: review?(dharvey)
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
Flags: qe-verify+
Priority: P4 → P3
QA Contact: brindusa.tot
Whiteboard: [reserve-photon-visual]
You need to log in before you can comment on or make changes to this bug.