1018078 - Extra 1px left border when the inspector pick icon is disabled

Status: RESOLVED DUPLICATE of bug 1226913

(DevTools :: Inspector, defect, P3)

Description

[Patrick Brosset <:pbro>]

Attachment: Screenshot 2014-05-30 10.24.32.png

In the latest nightly build (since bug 991810)

STR:
- open the toolbox options tab
- unselect the "pick an element from the page" toolbox button

==> The first panel tab (inspector) is moved to the left hand side of the toolbox. Therefore its 1px left border should be removed.
Right now it looks a little weird with this border there.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

filter on CLIMBING SHOES

Comment 2

[Florens Verschelde :fvsch]

Attachment: devtools-tabs-extra-border.png

Screenshot of this issue in Firefox 48 on OSX.

Comment 3

[Wellington Cordeiro [:wldcordeiro]]

I can take this if one of you would be willing to point me in the right direction for where in the codebase to look.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Wellington Cordeiro from comment #3)
> I can take this if one of you would be willing to point me in the right
> direction for where in the codebase to look.

Sure, thanks !

So the border width on the toolbox button is set here : https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/toolbars.css#891
The "Pick an element" button is set here : https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/toolbox.xul#116 and here https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/toolbox.js#1033. By default, the button is hidden, and stays so when the option is unchecked.

One solution might be to set a moz-border-start-width: 0 on the first child of .toolbox-tabs , and set a moz-border-end on toolbarbutton#command-button-pick .

Comment 5

[Wellington Cordeiro [:wldcordeiro]]

Thanks nchevobbe that should get me going just fine. You're welcome to assign me to this as I think I should have a patch in the next couple hours.

Comment 6

[Wellington Cordeiro [:wldcordeiro]]

Attachment: Bug1018078.patch

Comment 7

[Wellington Cordeiro [:wldcordeiro]]

Nicolas, I created the patch but on my platform (Ubunutu Linux 16.04) I can't really recreate the issue as shown in the screenshot Florent shared.

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8749888 [details] [diff] [review]
Bug1018078.patch

Review of attachment 8749888 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't fully reviewed the code, because I believe that we are removing the ability to disable this button in bug 1226913

::: devtools/client/themes/toolbars.css
@@ +897,5 @@
> +.devtools-tab:first-child {
> +  -moz-border-start-width: 0;
> +}
> +
> +toolbarbutton#command-button-pick {

Don't need the toolbarbutton portion of this selector

Comment 9

[Brian Grinstead [:bgrins]]

Nicolas / Patrick, please confirm that this is now a dupe of Bug 1226913

Comment 10

[Wellington Cordeiro [:wldcordeiro]]

Attachment: Bug1018078.patch

Comment 11

[Wellington Cordeiro [:wldcordeiro]]

(In reply to Brian Grinstead [:bgrins] from comment #8)
> Comment on attachment 8749888 [details] [diff] [review]
> Bug1018078.patch
> 
> Review of attachment 8749888 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I haven't fully reviewed the code, because I believe that we are removing
> the ability to disable this button in bug 1226913
> 
> ::: devtools/client/themes/toolbars.css
> @@ +897,5 @@
> > +.devtools-tab:first-child {
> > +  -moz-border-start-width: 0;
> > +}
> > +
> > +toolbarbutton#command-button-pick {
> 
> Don't need the toolbarbutton portion of this selector

Updated patch with that removed.

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Brian Grinstead [:bgrins] from comment #9)
> Nicolas / Patrick, please confirm that this is now a dupe of Bug 1226913

Indeed, if we remove the ability to disable the tool, this issue would not be visible. I was wondering if we have similar issue on the worker inspector, but it seems like not. Helen is currently working on the bug you linked so there's really no reason to fix this one now.
Wellington I'm sorry you had to work on this for nothing.