Closed Bug 981543 Opened 7 years ago Closed 7 years ago

[Australis] Adblock Plus button changes styling

Categories

(Firefox :: Extension Compatibility, defect)

All
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: pretzer, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P?])

Attachments

(1 file)

Attached image Screenshot of the issue
The toolbar button of Adblock Plus changes it's styling for me, when I enter customization mode. Before entering customization mode there is no vertical separator between the ABP icon and the dropdown arrow. After customization mode there is a separator between the two. See the screenshot.
Restarting the browser brings it back to the original styling (no separator).
In fact I just noticed that switching tabs also removes the separator. No restart necessary.
This isn't really a styling change, the button changes from type="menu" to type="menu-button". It's not immediately obvious to me why that happens, I'm quite certain that this used to work correctly.
Any idea what's causing this, Gijs?
Flags: needinfo?(gijskruitbosch+bugs)
The check |button.hasAttribute("context")| fails in the Adblock Plus code - does any Australis code remove that attribute and restore is asynchronously later?
Never mind, found it. wrapToolbarItem() in CustomizeMode.jsm will remove the context attribute, unwrapToolbarItem() will restore it. Adblock Plus updates the button's state from a progress listener - and that one notifies about a location change (customization tab closed) earlier than unwrapToolbarItem() runs.

The obvious hack would be to check for wrapped-context attribute in addition to context. I'd try to come up with something more reliable however.
Our fix has been pushed, I ended up removing the check for the context attribute altogether: https://hg.adblockplus.org/adblockplus/rev/3f165a815b57

Resolving as INVALID since there doesn't really seem to be a Mozilla issue involved here (unexpected event order doesn't qualify). Filed bug 982027 for a different issue that I noticed while testing this.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(gijskruitbosch+bugs)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.