Closed Bug 1297405 Opened 4 years ago Closed 4 years ago

"Blocked" permission icons in the identity block should not get a hover state

Categories

(Firefox :: Site Identity, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 51
Iteration:
51.2 - Aug 29
Tracking Status
firefox51 --- verified

People

(Reporter: johannh, Assigned: johannh)

References

Details

(Whiteboard: [fxprivacy] )

Attachments

(1 file)

Small regression from Bug 1022601.
Flags: qe-verify+
Comment on attachment 8783991 [details]
Bug 1297405 - Don't add hover state to blocked permission icons in the identity block.

Ugh, why are we reusing the notification-anchor-icon class for these? They aren't notification anchors.
Attachment #8783991 - Flags: review?(dao+bmo)
Iteration: --- → 51.2 - Aug 29
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Yeah good point, I didn't really think it was a problem since these were the applied the same styles but as we can see it can lead to misunderstandings once they need to behave differently.
Comment on attachment 8783991 [details]
Bug 1297405 - Don't add hover state to blocked permission icons in the identity block.

I think you can now get rid of the "blocked" class, too. Just use blocked-permission-icon instead.
Attachment #8783991 - Flags: review?(dao+bmo)
Comment on attachment 8783991 [details]
Bug 1297405 - Don't add hover state to blocked permission icons in the identity block.

Looks good, thanks!
Attachment #8783991 - Flags: review?(dao+bmo) → review+
https://hg.mozilla.org/integration/fx-team/rev/d51679a8d5588d992d9d710b67cb4e69c58298c7
Bug 1297405 - Don't add hover state to blocked permission icons in the identity block. r=dao
https://hg.mozilla.org/integration/fx-team/rev/15242d578a137703e4781a7595b9d3fa8e2a6072
Bug 1297405 followup - Remove unneeded classname assertion for blocked permission icons. r=me, bustage fix
(In reply to Johann Hofmann [:johannh] from comment #9)
> https://hg.mozilla.org/integration/fx-team/rev/
> 15242d578a137703e4781a7595b9d3fa8e2a6072
> Bug 1297405 followup - Remove unneeded classname assertion for blocked
> permission icons. r=me, bustage fix

Hmm, that check doesn't really seem unneeded to me. Shouldn't it just look for blocked-permission-icon instead?
Well the check is really unnecessary, because we're using dedicated icons that will always have the classname and blocked styling. If they're shown they will show as blocked.

I think this test wasn't fully updated from an old WIP patch where the notification anchors and the blocked icons where the same element. So yes, we should definitely update the selector to query for .blocked-permission-icon[data-permission-id=....].

Are you ok with making a new (good first?) bug for it? I wouldn't say solving this is super urgent, unless someone introduces a [data-permission-id] attribute somewhere else in the identity block.
Right, it's not clear at all that only "blocked permission" icons have the data-permission-id attribute. So I guess right now the check is unneeded, but it would make the test more future-proof.
See Also: → 1297693
https://hg.mozilla.org/mozilla-central/rev/d51679a8d558
https://hg.mozilla.org/mozilla-central/rev/15242d578a13
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
QA Contact: paul.silaghi
Depends on: 1298530
Reproduced on Nightly 2016-08-16.
Verified fixed FX 51.0a1 (2016-08-31) Win 7, Ubuntu 14.04, OS X 10.12.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.