Closed Bug 1561411 Opened 1 year ago Closed 1 year ago

Remove + report of add-on shows text for reporting a not removable add-on

Categories

(Toolkit :: Add-ons Manager, defect, P1, minor)

defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- verified
firefox70 --- verified

People

(Reporter: aryx, Assigned: rpl)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Firefox 68.0b12 and 69.0a1 20190625094022 on Windows 8.1

Remove + report of add-on shows text for reporting a not removable add-on.

These string entities are available:
abuse-report-messagebar-submitted-noremove
abuse-report-messagebar-removed-extension
abuse-report-messagebar-removed-theme

Steps to reproduce:

  1. Launch Firefox.
  2. Install an add-on (doesn't matter if it needs some permissions or not).
  3. Open the Add-ons Manager (menu Tools)
  4. Click on the "..." next to the add-on to open its menu.
  5. Select "Remove".
  6. Choose to report it.
  7. Pick "Something else".
  8. Enter "Test, ignore".
  9. Submit.

Actual result: "Thank you for submitting a report." shown

Expected result:
One of those shown:
+abuse-report-messagebar-removed-extension = Thank you for submitting a report. You’ve removed the extension <span data-l10n-name="addon-name">{ $addon-name }</span>.
+abuse-report-messagebar-removed-theme = Thank you for submitting a report. You’ve removed the theme <span data-l10n-name="addon-name">{ $addon-name }</span>.

https://searchfox.org/mozilla-beta/rev/b9194766504b41d7bb1f2a1ce4119f59bf63a750/toolkit/mozapps/extensions/content/abuse-reports.js#144
The comment above mentions the message shall only be shown if the addon cannot be removed.

Assignee: nobody → lgreco
Blocks: 1540175
Severity: normal → minor
Status: NEW → ASSIGNED
Priority: -- → P2
Regressed by: 1549991

If we're actively working on this, shouldn't it be a P1?

Flags: needinfo?(lgreco)
Priority: P2 → P1

Sure, P1 sounds to me too, it has also already a patch in review.

(I originally marked it as P2 mostly because it wasn't a blocking issue for the abuse report feature in 68, the message currently shown is still valid but it is not the message we wanted to show in that scenario, and so this fix wasn't one of those we needed to land sooner so that we could also ask for an uplift to beta).

Flags: needinfo?(lgreco)
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/2d080c3aa702
Fix abuse report message bar string shown on report on addon uninstall. r=mstriemer
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Verified the fix with the latest Nightly (70.0a1 / 20190714214027) under Windows 10 Pro 64-bit and macOS High Sierra 10.13.6.

Removing and reporting an add-on (extension or theme) will display the appropriate message bars:

  • Thank you for submitting a report. You’ve removed the extension <name of the extension>
  • Thank you for submitting a report. You’ve removed the theme <name of the theme>

Closing the issue as Verifed.

Status: RESOLVED → VERIFIED

Please nominate this for Beta and ESR68 approval when you get a chance.

Flags: needinfo?(lgreco)

Comment on attachment 9074131 [details]
Bug 1561411 - Fix abuse report message bar string shown on report on addon uninstall. r?mstriemer!

Beta/Release Uplift Approval Request

  • User impact if declined: The message bar shown after submitting an abuse report started on add-on removal does not mention the removed extension name.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Same STR used to verify the issue on Nightly (Bug 1561411 comment 0)
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): small change, QA verified explicitly on Nightly.
    The change did baked for a while on Nightly, and it is also tested explicitly as part of the automated tests.
  • String changes made/needed:
Flags: needinfo?(lgreco)
Attachment #9074131 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9074131 [details]
Bug 1561411 - Fix abuse report message bar string shown on report on addon uninstall. r?mstriemer!

Fixes the abuse report message bar so that the name of the removed addon is shown. Approved for 69.0b8.

Attachment #9074131 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified the fix with the latest Beta (69.0b8 / 20190725174626) under Windows 10 Pro 64-bit and macOS High Sierra 10.13.6.

Removing and reporting an add-on (extension or theme) will display the appropriate message bars:

  • Thank you for submitting a report. You’ve removed the extension <name of the extension>
  • Thank you for submitting a report. You’ve removed the theme <name of the theme>

Is this something we should take on ESR68 also?

Flags: needinfo?(lgreco)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)

Is this something we should take on ESR68 also?

Not strictly necessary (as mentioned in comment 3 the message currently shown is still valid, but it is not the message we wanted to show in that scenario), but it is not a big change and it should be low risk one, and so we may as well create an uplift request to ESR for it.

It may be reasonable to ask it along with uplift requests to ESR for Bug 1556403 and Bug 1556757.

I'll create the uplift requests for these 3 patches asap.

Flags: needinfo?(lgreco)
Flags: needinfo?(lgreco)

I took a look and Bug 1556403 and Bug 1556757 patches need to be applied on top of Bug 1562792 patch (because they also introduce small changes, additional assertions, to tests that were also changed in Bug 1562792).

Bug 1562792 patch would need to be slightly changed to apply cleanly on ESR68 (because the browser_html_abuse_report.js test file wasn't skipped on windows in 68 and so the patch has conflicts when applied as it is).

On the contrary Bug 1561411 patch does apply cleanly on ESR68 without any further changes.

RyanVM,
I'm going to request an uplift for just this one for now, let me know if we are interested in creating uplift requests for the other ones too (in that case I'll attach a modified version of the Bug 1562792 patch for ESR68).

Flags: needinfo?(lgreco)

Comment on attachment 9074131 [details]
Bug 1561411 - Fix abuse report message bar string shown on report on addon uninstall. r?mstriemer!

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: As described in the User impact if declined the message shown when an abuse report is being triggered from the "addon removal flow" is not mentioning the extension name of the "reported + removed" addon, and so the message shown is not the one we wanted to show in that scenario.

It is not a blocking issue (because the abuse report is still being submitted and the message shown is still valid, even if missing the mention to the extension name of the addon reported and removed),
but it may still be reasonable to fix it on ESR68 (as it is going to be around longer than the regular 68 release).

  • User impact if declined: Same as the one mentioned in the uplift request to beta:
    The message bar shown after submitting an abuse report started on add-on removal does not mention the removed extension name.
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): small change, QA verified it explicitly on Nightly, as well as on Beta (after it has been uplifted to beta 69, see comment 11).

The change baked for a while on Nightly and Beta, and it is also tested explicitly as part of the automated tests.

  • String or UUID changes made by this patch:
Attachment #9074131 - Flags: approval-mozilla-esr68?

Comment on attachment 9074131 [details]
Bug 1561411 - Fix abuse report message bar string shown on report on addon uninstall. r?mstriemer!

I think I'll call this wontfix for esr68, I'm not convinced it's worth it.

Attachment #9074131 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68-
You need to log in before you can comment on or make changes to this bug.