Button image does not fit into button in Responsive Design Mode

VERIFIED FIXED in Firefox 54

Status

P1
normal
VERIFIED FIXED
2 years ago
3 months ago

People

(Reporter: magicp.jp, Assigned: ntim)

Tracking

({regression})

Trunk
Firefox 54
regression

Firefox Tracking Flags

(firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 verified)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Reporter)

Description

2 years ago
Created attachment 8839156 [details]
button-image-does-not-fit-into-button-in-responsive-design-mode.png

[Steps to reproduce]
1. Start latest Nightly (e.g. Build Id: 20170219030209)
2. Open Responsive Design Mode (Ctrl+Shift+M)
3. Confirm position of button images

[Actual Results]
Button image does not fit into button.

[Expected Results]
Button image fits into button.
(Reporter)

Updated

2 years ago
Version: 54 Branch → Trunk
(Reporter)

Comment 1

2 years ago
Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=813c8c4770bc41bd69457e57e62b2bbe9ffeb855&tochange=6eb03c463039bb62bdfe4bf66cea8144c7cb83c0
Has Regression Range: --- → yes
status-firefox53: --- → unaffected
Depends on: 1255116
(Reporter)

Updated

2 years ago
Blocks: 1255116
No longer depends on: 1255116
:ntim, could you look into this?
Flags: needinfo?(ntim.bugs)
Keywords: regression
Priority: -- → P1
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Flags: needinfo?(ntim.bugs)
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED

Comment 6

2 years ago
mozreview-review
Comment on attachment 8841223 [details]
Bug 1341045 - Make sure RDM close icon is centered within button.

https://reviewboard.mozilla.org/r/115502/#review117106

Thanks, this deduplcation seems fine.
Attachment #8841223 - Flags: review?(jryans) → review+

Comment 7

2 years ago
mozreview-review
Comment on attachment 8841221 [details]
Bug 1341045 - Remove broken CSS overrides from responsive.html.

https://reviewboard.mozilla.org/r/115498/#review117108

Thanks for working on this!  It's getting close, but I think a few more changes are needed to restore previous appearance.

::: devtools/client/responsive.html/index.css:74
(Diff revision 1)
>  .container {
>    background-color: var(--theme-toolbar-background);
>    border: 1px solid var(--theme-splitter-color);
>  }
>  
>  .toolbar-button {

`.devtools-button` now gets a background color on hover, which is different than previous behavior.  I think the RDM buttons don't want this (at least for this bug, let's just aim to restore previous functionality).

So, we'll need a rule for something like `.toolbar-button:hover` to clear the background, but it needs to override the `.devtools-button:empty:hover:not(:disabled)` rule.

The same thing happens with hover when checked, so another rule seems to be needed to clear that as well.

::: devtools/client/responsive.html/index.css
(Diff revision 1)
>  }
>  
>  .toolbar-button {
> -  margin: 1px 3px;
> -  width: 16px;
> +  margin: 0;
> +  padding: 0;
> -  height: 16px;

It seems like there are more styles needed to restore the previous appearance:

```
border: none;
```

After adding this, it feels like the viewport rotate button is no longer vertically centered...  Maybe reduce the height on `.viewport-toolbar` by 2px to 16px?
Attachment #8841221 - Flags: review?(jryans) → review-

Comment 8

2 years ago
mozreview-review
Comment on attachment 8841222 [details]
Bug 1341045 - Use checked class instead of introducing active class.

https://reviewboard.mozilla.org/r/115500/#review117130

Thanks, this seems like good simplification.
Attachment #8841222 - Flags: review?(jryans) → review+
Summary: Button image dose not fit into button in Responsive Design Mode → Button image does not fit into button in Responsive Design Mode
:ntim, it would be great to fix this before the release on Monday.  Do you have time to work on it, or should I finish it up?
Flags: needinfo?(ntim.bugs)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #9)
> :ntim, it would be great to fix this before the release on Monday.  Do you
> have time to work on it, or should I finish it up?

It would be great if you could finish it up. I don't have time this month.
Flags: needinfo?(ntim.bugs)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8841221 - Attachment is obsolete: true
Attachment #8841222 - Attachment is obsolete: true
Attachment #8841223 - Attachment is obsolete: true

Comment 14

2 years ago
mozreview-review
Comment on attachment 8843372 [details]
Bug 1341045 - Make sure RDM close icon is centered within button.

https://reviewboard.mozilla.org/r/117136/#review118744
Attachment #8843372 - Flags: review?(jryans) → review+

Comment 15

2 years ago
mozreview-review
Comment on attachment 8843371 [details]
Bug 1341045 - Use checked class instead of introducing active class.

https://reviewboard.mozilla.org/r/117134/#review118748
Attachment #8843371 - Flags: review?(jryans) → review+

Comment 16

2 years ago
mozreview-review
Comment on attachment 8843370 [details]
Bug 1341045 - Remove broken CSS overrides from responsive.html.

https://reviewboard.mozilla.org/r/117132/#review118750
Attachment #8843370 - Flags: review?(jryans) → review+

Comment 17

2 years ago
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f67c47f25e6c
Remove broken CSS overrides from responsive.html. r=jryans
https://hg.mozilla.org/integration/autoland/rev/a8457e6674a2
Use checked class instead of introducing active class. r=jryans
https://hg.mozilla.org/integration/autoland/rev/cb3ac86ceeb8
Make sure RDM close icon is centered within button. r=jryans

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f67c47f25e6c
https://hg.mozilla.org/mozilla-central/rev/a8457e6674a2
https://hg.mozilla.org/mozilla-central/rev/cb3ac86ceeb8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
status-firefox52: --- → unaffected
status-firefox-esr52: --- → unaffected
I have reproduced this bug with Nightly 54.0a1 (2017-02-20) (64-bit) on WIndows 7 , 64 Bit!

This bug's fix is verified with latest Developer Edition (Aurora)!

Build ID          20170309004016
User Agent        Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0

[bugday-20170308]
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified

Comment 20

a year ago
[bugday-20170419]

Status: Fixed and verified
Browser: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0

The issue is no longer reproducible on Firefox Aurora 
Tests were performed in Ubuntu 16.04.2 x64

Updated

3 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.