Remove UITour.showHeartbeat

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mythmon, Assigned: mythmon)

Tracking

unspecified
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Once SelfSupport is gone (bug 1361578), we can remove UITour.showHeartbeat, since it doesn't have any other users, and is replaced by Shield.
(Assignee)

Updated

2 years ago
Depends on: 1366006
(Assignee)

Updated

2 years ago
Assignee: nobody → mcooper
Comment hidden (mozreview-request)
Comment on attachment 8869606 [details]
Bug 1366005 - Remove UITour.showHeartbeat.

https://reviewboard.mozilla.org/r/141192/#review144770

Thank for taking this :)

::: browser/components/uitour/UITour.jsm:16
(Diff revision 1)
>  Cu.import("resource://gre/modules/AppConstants.jsm");
>  Cu.import("resource://gre/modules/Services.jsm");
>  Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>  Cu.import("resource://gre/modules/Preferences.jsm");
>  Cu.import("resource://gre/modules/Promise.jsm");
>  Cu.import("resource:///modules/RecentWindow.jsm");

You can remove this import now too I think

::: browser/components/uitour/UITour.jsm:40
(Diff revision 1)
>    "resource:///modules/ReaderParent.jsm");
>  
>  // See LOG_LEVELS in Console.jsm. Common examples: "All", "Info", "Warn", & "Error".
>  const PREF_LOG_LEVEL      = "browser.uitour.loglevel";
>  const PREF_SEENPAGEIDS    = "browser.uitour.seenPageIDs";
>  const PREF_SURVEY_DURATION = "browser.uitour.surveyDuration";

PREF_SURVEY_DURATION can be deleted I think… eslint no-unused should have caught that I guess

::: browser/themes/linux/browser.css
(Diff revision 1)
> -%include ../shared/UITour.inc.css
> -

You definitely don't want to be deleting this CSS file… just remove the heartbeat-specific stuff
Attachment #8869606 - Flags: review?(MattN+bmo)
Status: NEW → ASSIGNED
Component: General → Tours
Comment hidden (mozreview-request)
Comment on attachment 8869606 [details]
Bug 1366005 - Remove UITour.showHeartbeat.

https://reviewboard.mozilla.org/r/141192/#review145328

Thanks

::: browser/themes/shared/UITour.inc.css
(Diff revision 2)
> -.heartbeat > #star-rating-container > #star2 {
> -  -moz-box-ordinal-group: 2;
> -}
> -
> -.heartbeat > #star-rating-container > .star-x  {
> -  background: url("chrome://browser/skin/heartbeat-star-off.svg");

I assume you are still going to use this and the 2 other svg files with SHIELD?
Attachment #8869606 - Flags: review?(MattN+bmo) → review+
(Assignee)

Comment 5

2 years ago
> I assume you are still going to use this and the 2 other svg files with SHIELD?

Actually, that's an oversight. I don't see any other in-tree code using these svgs. I'll copy those svgs over to the SHIELD add-on as well and remove them here.
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Depends on: 1367598
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 9

2 years ago
I've added a commit that adds the removed CSS to shield-recipe-client. Originally I was going to do this separately in bug 
1366006, but I wasn't able to get them to work separately. The second patch was reviewed by Gijs on Github: https://github.com/mozilla/normandy/pull/770.
(Assignee)

Updated

2 years ago
Attachment #8871341 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 10

2 years ago
Comment on attachment 8871341 [details]
Bug 1366005 - Move UITour CSS into shield-recipe-client.

I was hoping that by setting r=Gijs here, it would transfer to Reviewboard (it did not).

NB: Gijs approved that patch on Github, which is why I set r+ for him.
Attachment #8871341 - Flags: review+ → review?(gijskruitbosch+bugs)

Comment 11

2 years ago
mozreview-review
Comment on attachment 8871341 [details]
Bug 1366005 - Move UITour CSS into shield-recipe-client.

https://reviewboard.mozilla.org/r/142818/#review146846
Attachment #8871341 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 12

2 years ago
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc2c82d8ba22
Remove UITour.showHeartbeat r=MattN
https://hg.mozilla.org/integration/autoland/rev/7662d766f166
Move UITour CSS into shield-recipe-client. r=Gijs
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 years ago
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48a6570b38cb
Remove UITour.showHeartbeat. r=MattN
https://hg.mozilla.org/integration/autoland/rev/9cdcc98ab24a
Move UITour CSS into shield-recipe-client. r=Gijs

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/48a6570b38cb
https://hg.mozilla.org/mozilla-central/rev/9cdcc98ab24a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Heads-up: this bug (specifically, moving the "heartbeat > #star-rating-container > .star-x" CSS rules) caused breakage in the Pulse addon, which seems to have been sharing the star styling & images that this bug moved around.

I reported that issue here:
  https://github.com/mozilla/pulse/issues/179

(Not sure at this point whether the fix will be entirely in the Pulse addon itself vs. whether there are mozilla-central changes needed as well. Anyway, just noting the connection here, for the record.)
You need to log in before you can comment on or make changes to this bug.