UITour info panels have incorrect top padding

VERIFIED FIXED in Firefox 48

Status

()

P4
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: agibson, Assigned: jaws)

Tracking

({regression})

48 Branch
Firefox 50
regression
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox47 unaffected, firefox48+ verified, firefox49+ verified, firefox50 verified)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8749068 [details]
info-panel.png

"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0"

STR:

1.) Using Firefox 48.0a2, click Help -> Firefox Tour
2.) Wait for the mozilla.org page to load and the first tour panel to open

Expected results:

Info panel should have padding between the top of the panel and the panel content.

Actual results:

The top padding is missing, which looks odd.
Perhaps related to bug 1180186? Or maybe the devedition theme?

Can someone run mozregression?
Keywords: regressionwindow-wanted
(Reporter)

Comment 2

2 years ago
Note: I can see the same issue in Nightly, so it doesn't seem to be related to the Dev Edition theme.
[Tracking Requested - why for this release]: Regression with UI shown on firstrun and what's new in some versions.

A regression range would still be good.
status-firefox47: --- → unaffected
status-firefox48: --- → affected
status-firefox49: --- → affected
tracking-firefox48: --- → ?
Keywords: qawanted, regression
Priority: -- → P1
(Reporter)

Comment 4

2 years ago
I'll see if I can find some time to run mozregression next week if no one else gets to it first.
I've managed to reproduce this issue on Windows 7 x32 on the latest Nightly(49.0a1) and on the latest Aurora(48.0a2). I've performed a regression and below are my results:

Last good revision: 8975734f1546f7ab475037441e9a0591af299e32
First bad revision: 1306c789088d5675435586edf04b9a2b70f86dec
Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=8975734f1546f7ab475037441e9a0591af299e32&tochange=1306c789088d5675435586edf04b9a2b70f86dec

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1180186
Keywords: qawanted, regressionwindow-wanted
Blair, can you take this?
Flags: needinfo?(bmcbride)
Let's track this for both 48/49.
tracking-firefox48: ? → +
tracking-firefox49: --- → +
Created attachment 8753049 [details] [diff] [review]
some simple changes to make the margin/padding more uniform

This doesn't look as bad on Windows, though the margins could still be tweaked a little.
Assignee: nobody → jaws
Flags: needinfo?(bmcbride)
Priority: P1 → P4
Comment on attachment 8753049 [details] [diff] [review]
some simple changes to make the margin/padding more uniform

Hmm, not sure if Jared meant to flag this for review or not. (And he's out this week.)

Dao, you want to weigh in with a review/feedback comment here anyway?
Attachment #8753049 - Flags: review?(dao+bmo)
Comment on attachment 8753049 [details] [diff] [review]
some simple changes to make the margin/padding more uniform

This bug was filed about the top padding (technically a margin) but this patch doesn't touch that. Should we change margin-top: -16px; closer to zero? Also, would be nice if we could avoid making margin-inline-end and margin-inline-start negative.
Attachment #8753049 - Flags: review?(dao+bmo)
The build for beta 7 is already started, but a patch here could still make it into beta 8 later this week.
Flags: needinfo?(jaws)
Created attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

Review commit: https://reviewboard.mozilla.org/r/63582/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63582/
Attachment #8769917 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

https://reviewboard.mozilla.org/r/63582/#review60586

http://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#2559

We're not shipping the CSS3 grid model, nor can I find an intent to ship email in m.d.platform, so I don't think we can do this.
Attachment #8769917 - Flags: review?(gijskruitbosch+bugs) → review-
Comment on attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63582/diff/1-2/
Attachment #8769917 - Flags: review- → review?(gijskruitbosch+bugs)
Attachment #8753049 - Attachment is obsolete: true
Flags: needinfo?(jaws)
Comment on attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63582/diff/2-3/
I tested on Windows 10 and Linux Mint. Can you test the patch on OSX?
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

https://reviewboard.mozilla.org/r/63582/#review60964

Looks OK to me on OS X. Tested with the reader mode tooltip (plus manually changing some text in there to see how it behaved in different circumstances).
Attachment #8769917 - Flags: review?(gijskruitbosch+bugs) → review+
Flags: needinfo?(gijskruitbosch+bugs)

Comment 19

2 years ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3f0ce20c16c1
Simplify the markup and padding rules for the UI Tour panel. r=Gijs
Duplicate of this bug: 1209202
Thanks, I tested with the Reader mode tooltip as well as the Firefox tour (Help > Nightly Tour).

Comment 22

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3f0ce20c16c1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Comment on attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

Approval Request Comment
[Feature/regressing bug #]: followup to bug  	1180186
[User impact if declined]: ugly UITour panels and reader view intro panel on Windows
[Describe test coverage new/current, TreeHerder]: tested visually on Windows, Linux, and OSX
[Risks and why]: low risk, only affects UITour panel styling
[String/UUID change made/needed]: none
Attachment #8769917 - Flags: approval-mozilla-beta?
Attachment #8769917 - Flags: approval-mozilla-aurora?
Comment on attachment 8769917 [details]
Bug 1270395 - Simplify the markup and padding rules for the UI Tour panel.

This patch fixes a UI regression. Take it in 48 beta 8 and aurora.
Attachment #8769917 - Flags: approval-mozilla-beta?
Attachment #8769917 - Flags: approval-mozilla-beta+
Attachment #8769917 - Flags: approval-mozilla-aurora?
Attachment #8769917 - Flags: approval-mozilla-aurora+

Comment 25

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/6463668502ef
status-firefox49: affected → fixed
has problems to apply to beta:

merging browser/themes/shared/UITour.inc.css
warning: conflicts while merging browser/themes/shared/UITour.inc.css! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(jaws)
Rebased, tested, and pushed to mozilla-beta,
https://hg.mozilla.org/releases/mozilla-beta/rev/8139a58cc0c1
Flags: needinfo?(jaws)
status-firefox48: affected → fixed
Flags: qe-verify+
I've reproduced the initial issue on 48.0a2 (2016-05-05). 

Verified fixed on Windows 7 64bit, Ubuntu 14.04 64bit and Mac OSX 10.9.5 using latest Nightly 50.0a1 (buildID: 20160720030208), latest Aurora 49.0a2 (buildID: 20160721004019) and Firefox 48 Beta 9 (buildID: 20160718142219).
Status: RESOLVED → VERIFIED
status-firefox48: fixed → verified
status-firefox49: fixed → verified
status-firefox50: fixed → verified
You need to log in before you can comment on or make changes to this bug.