Closed Bug 1366005 Opened 7 years ago Closed 7 years ago

Remove UITour.showHeartbeat

Categories

(Firefox :: Tours, enhancement)

Product:
Firefox
Component:
Tours
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: mythmon, Assigned: mythmon)

References

Details

Attachments

(2 files)

Bug 1366005 - Remove UITour.showHeartbeat.
59 bytes, text/x-review-board-request
MattN
: review+
Details
Bug 1366005 - Move UITour CSS into shield-recipe-client.
59 bytes, text/x-review-board-request
Gijs
: review+
Details 
Once SelfSupport is gone (bug 1361578), we can remove UITour.showHeartbeat, since it doesn't have any other users, and is replaced by Shield.
Depends on: 1366006
Assignee: nobody → mcooper
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 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+
> 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.
Depends on: 1367598
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.
Attachment #8871341 - Flags: review?(gijskruitbosch+bugs) → review+
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 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+
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
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
https://hg.mozilla.org/mozilla-central/rev/48a6570b38cb
https://hg.mozilla.org/mozilla-central/rev/9cdcc98ab24a
Status: ASSIGNED → RESOLVED
Closed: 7 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.

Attachment

General

Creator:
Created:
Updated:
Size: