User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 Build ID: 20160607071209 Steps to reproduce: 1. Start Nightly with the following add-ons - Adblock Plus "https://addons.mozilla.org/firefox/addon/adblock-plus" - Element hiding helper for Adblock Plus "https://addons.mozilla.org/firefox/addon/elemhidehelper" 2. Open devtools > Inspector Actual results: Breadcrumbs overlaps on the element hiding helper button. In 48.0b1, It does not overlap on that. Expected results: Don't overlap.

Regression range: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=e3d409d10dea7c1c94630abf52e6e9245a77f448&tochange=5d2cb7fff024a1d3ed59ad2432882d158ce5ee91

This extension adds a (XUL) toolbarbutton inside #inspector-breadcrumbs-toolbar before the #inspector-breadcrumbs element. However, the #inspector-breadcrumbs element is now position: absolute to work around a XUL/html interaction, thus the overlap. (See Bug 1259812 Comment 18)

WIP: With following change the icon could show without overlapping, Though the left padding(20px) of `.breadcrumbs-widget-item` shows a large gap. > --- a/devtools/client/themes/inspector.css > +++ b/devtools/client/themes/inspector.css > @@ -37,7 +37,8 @@ > padding: 0px; > border-bottom-width: 0px; > border-top-width: 1px; > - display: block; > + display: flex; > + flex-direction: row; > position: relative; > } > > @@ -52,7 +53,7 @@ > /* Break out of the XUL flexbox, so the splitter can still shrink the > markup view even if the contents of the breadcrumbs are wider than > the new width. */ > - position: absolute; > + position: relative; > top: 0; > left: 0;

Review commit: https://reviewboard.mozilla.org/r/63392/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/63392/

adblock plus add the toolbox-button xul before the element with `breadcrumbs-widget-container` class. Besides the change mentioned in comment 3, I compensate the first breadcrumbs-widget-item's left padding when breadcrumbs-widget-container is not the first element https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/widgets.css#246

Comment on attachment 8769540 [details] Bug 1278774 - Breadcrumbs overlaps abp helper button; https://reviewboard.mozilla.org/r/63392/#review60386 ::: devtools/client/themes/inspector.css:40 (Diff revision 1) > > #inspector-breadcrumbs-toolbar { > padding: 0px; > border-bottom-width: 0px; > border-top-width: 1px; > - display: block; > + display: flex; I don't know enough about the interaction between CSS flex and XUL flex to say if doing <parent style='display: flex'><child style='position: relative'> sufficiently 'breaks out' in the same way that this does: <parent style='display: block'><child style='position: absolute'> But it seems to not change things for me locally so should be OK, I think. Going to forward this on for Patrick to take a look to make sure he's aware of this potential change ::: devtools/client/themes/widgets.css:127 (Diff revision 1) > } > > +/* Bug 1278774 - compenssate first breadcrumbs-widget-item's left padding when > + breadcrumbs-widget-container is not the first element */ > +:not(.breadcrumbs-widget-container) + .breadcrumbs-widget-container { > + margin-inline-start: -20px; Not sure I understand why this needed. I think it's OK if there's a space between the ABP button and the first breadcrumb (if that's all this is preventing I don't think we need to special case it for the addon).

> Not sure I understand why this needed. I think it's OK if there's a space > between the ABP button and the first breadcrumb (if that's all this is > preventing I don't think we need to special case it for the addon). Yes if a space between the ABP button and the first breadcrumb is OK, I can remove that style.

Comment on attachment 8769540 [details] Bug 1278774 - Breadcrumbs overlaps abp helper button; https://reviewboard.mozilla.org/r/63392/#review61888 I'm not comfortable changing the positioning scheme of the breadcrumbs because we are (at least for now) in a mixed XUL+HTML world where layout is weird. The absolute positioning of the breadcrumbs inside its toolbar is nice because it works fine and allows us to break out of the XUL flex layout mechanism. As noted below in my comments, this change breaks one of the key features of the breadcrumbs widget: scrolling. Instead of finding a way to accomodate the ABP button automatically in our CSS, I think it would be much easier for the addon to override one CSS property: ``` #inspector-breadcrumbs { left: 30px; } ``` 30px or whatever width is the button. Only pushing the left edge of the breadcrumbs works fine and allows to preserve our absolute positioning. So, we should either reach out to the authors and see if this is a change they can make, or we can have this as a rule in our CSS in case the #inspector-breadcrumbs isn't first child in #inspector-breadcrumbs-toolbar, but if we do this, then we'd have to choose and hard code a width. ::: devtools/client/themes/inspector.css:41 (Diff revision 1) > #inspector-breadcrumbs-toolbar { > padding: 0px; > border-bottom-width: 0px; > border-top-width: 1px; > - display: block; > + display: flex; > + flex-direction: row; `row` is the default `flex-direction` anyway, so no need to define it here. ::: devtools/client/themes/inspector.css:56 (Diff revision 1) > - position: absolute; > + position: relative; > top: 0; > left: 0; > bottom: 0; > right: 0; This change breaks the breadcrumbs autoscrolling. Now the `#inspector-breadcrumbs` element does not fit into the available space, and instead seems to expand to all the width it requires, and therefore there are no arrows to scroll through the breadcrumbs anymore. So we need to find another solution. ::: devtools/client/themes/widgets.css:126 (Diff revision 1) > +:not(.breadcrumbs-widget-container) + .breadcrumbs-widget-container { > + margin-inline-start: -20px; > +} Agreed with Brian, don't think this is necessary.

Thanks for feedback! According to comment 8, the proper solution might be add `#inspector-breadcrumbs` style in the addon (Element hiding helper for Adblock Plus) directly. ``` #inspector-breadcrumbs { left: 30px; } ``` Honza, could you help redirect the request to Wladimir Palant(ABP author)?

Wladimir, can you please help out with this bug? See comment #9 Honza

(In reply to Fred Lin [:gasolin] from comment #9) > According to comment 8, the proper solution might be add > `#inspector-breadcrumbs` style in the addon (Element hiding helper for > Adblock Plus) directly. > > ``` > #inspector-breadcrumbs { > left: 30px; > } > ``` That's something I would consider a very bad idea. It's bad enough that Element Hiding Helper has to make assumptions about the Dev Tools layout - this is what backfired here. Making assumptions about styling is worse, this is bound to break things even with some themes, and future Firefox versions are guaranteed to mess everything up. Either way, looking at the new layout it makes much more sense to have our button after the inspector-element-add-button element if it exists (meaning Firefox 48 and higher). Having the button in the breadcrumbs toolbar was never the intention. I filed https://issues.adblockplus.org/ticket/4255 on this, will put a patch under review there soon.

Our fix has landed. As far as I am concerned this can be resolved as INVALID.

Thanks Wladimir!