1417273 - The "Report Site Issue" page action can be disabled in the page action menu but enabled in the urlbar

Status: RESOLVED FIXED

(Web Compatibility :: Tooling & Investigations, enhancement)

Description

[Drew Willcoxon :adw]

If you open a page that's not compatible with the webcompat reporter (about:about for example), the Report Site Issue menu item in the page action menu will be disabled.  But if you also add that action to the urlbar, its button there will not be disabled.  The menu item is disabled here: https://dxr.mozilla.org/mozilla-central/rev/fc194660762d1b92e1679d860a8bf41116d0f54f/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#47

Only the node in the menu is disabled, not the one in the urlbar (if present).  What should actually happen now (probably) is to call PageActions.actionForID("webcompat-reporter-button").setDisabled(window), now that we have that ability.  That will automatically disable both nodes for that window.

Comment 1

[Mike Taylor [:miketaylr]]

Thanks for reporting! (And for the suggested fix...). Will work on a patch tonight.

Comment 2

[Drew Willcoxon :adw]

Thanks Mike.  The patch here should probably be similar to the patch in bug 1417272:  Replace the onShowingInPanel handler with an onLocationChange handler, and then make the call to setDisabled() in that handler.

Comment 3

[Mike Taylor [:miketaylr]]

Attachment: Bug 1417273. Use PageAction.setDisabled to enable/disable Report Site Issue button.

Review commit: https://reviewboard.mozilla.org/r/200452/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200452/

Comment 4

[Mike Taylor [:miketaylr]]

Attachment: Bug 1417273. Remove obsolete assertion from Report Site Issue test. , a=testonly

Review commit: https://reviewboard.mozilla.org/r/200454/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200454/

Comment 5

[Mike Taylor [:miketaylr]]

Comment on attachment 8929165 [details]
Bug 1417273. Remove obsolete assertion from Report Site Issue test. , a=testonly

https://reviewboard.mozilla.org/r/200454/#review205570

Comment 6

[Drew Willcoxon :adw]

Comment on attachment 8929164 [details]
Bug 1417273. Use PageAction.setDisabled to enable/disable  Report Site Issue button.

https://reviewboard.mozilla.org/r/200452/#review205600

Looks good, thanks!

Comment 7

[Mike Taylor [:miketaylr]]

Thanks for review!

Comment 8

[Mike Taylor [:miketaylr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9755fd889aa9832fbbd8cba6fb2c7952879aab33

Comment 9

[Mike Taylor [:miketaylr]]

Comment on attachment 8929164 [details]
Bug 1417273. Use PageAction.setDisabled to enable/disable  Report Site Issue button.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200452/diff/1-2/

Comment 10

[Mike Taylor [:miketaylr]]

Comment on attachment 8929165 [details]
Bug 1417273. Remove obsolete assertion from Report Site Issue test. , a=testonly

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200454/diff/1-2/

Comment 11

[Mike Taylor [:miketaylr]]

(oops, had an extra space making eslint sad -- fixed now)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ca73dc22a1f
Use PageAction.setDisabled to enable/disable  Report Site Issue button. r=adw
https://hg.mozilla.org/integration/autoland/rev/16c55e718c19
Remove obsolete assertion from Report Site Issue test. r=miketaylr, a=testonly

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/2ca73dc22a1f
https://hg.mozilla.org/mozilla-central/rev/16c55e718c19