1508169 - Remove performAction, performActionOnRow and performActionOnCell methods from tree interface

Status: RESOLVED FIXED

(Core :: XUL, task, P5)

Description

[Tim Nguyen :ntim]

performActionOnRow appears to be only used once: https://searchfox.org/mozilla-central/rev/5117a4c4e29fcf80a627fecf899a62f117368abf/browser/base/content/pageinfo/pageInfo.js#80-89

...and that usage actually seems broken. The copy context menu always appears disabled and the keyboard shortcut also does not work.


performAction, performActionOnCell actually seem unused everywhere.


I haven't checked for comm-central.

Comment 1

[Alex Vincent [:WeirdAl]]

https://searchfox.org/comm-central/search?q=performAction&path=
It's not used there either, except in one place: https://searchfox.org/comm-central/source/suite/browser/pageinfo/pageInfo.js#85
That one use matches what comment 0 says.

Comment 2

[Alex Vincent [:WeirdAl]]

More to the point, C++ code never calls it:
https://searchfox.org/mozilla-central/search?q=PerformAction&case=false&regexp=false&path=.cpp%24

Comment 3

[Alex Vincent [:WeirdAl]]

https://searchfox.org/comm-central/rev/6e6533927f58e5bce51970872bb9973a480460eb/suite/mailnews/content/msgMail3PaneWindow.js#1032
nsIXULTreeBuilder is not defined anywhere in our tree, so something needs to be done about that.

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: bug1508169-mozilla-central.diff

Comment 5

[Alex Vincent [:WeirdAl]]

Attachment: bug1508169-comm-central.diff

These patches are currently untested, but are also as conservative as they can be: I only removed lines that I felt were safe.

Comment 6

[Alex Vincent [:WeirdAl]]

Well, this is embarrassing: I can't seem to set a review? flag anymore. It's been quite a while since I submitted patches to Bugzilla. Help?

Comment 7

[Alex Vincent [:WeirdAl]]

Comment on attachment 9076978 [details] [diff] [review]
bug1508169-mozilla-central.diff

The mozilla-central patch didn't compile:  I missed TreeView.webidl.  New patch coming once I do have a compiled build.

Comment 8

[Alex Vincent [:WeirdAl]]

Attachment: bug1508169-mozilla-central.diff

Comment 9

[Alex Vincent [:WeirdAl]]

Attachment: Bug 1508169, Remove performAction* from nsITreeView.idl in mozilla-central. r=peterv, johannh

performAction, performActionOnRow and performActionOnCell are methods of the
nsITreeView interface that are never called. This is to remove these methods.
A comm-central patch will be along shortly.

Comment 10

[Alex Vincent [:WeirdAl]]

Attachment: Bug 1508169, Remove performAction* from nsITreeView.idl in comm-central, r=jorgk

performAction, performActionOnRow and performActionOnCell are methods of the
nsITreeView interface that are never called. This is to remove these methods.
The mozilla-central patch is https://phabricator.services.mozilla.com/D39273.

Comment 11

[Frank-Rainer Grahl (:frg)]

Note that code in suite/ is for SeaMonkey. Surely the removals are OK, I don't know what happens in suite/browser/pageinfo/pageinfo.js, getSelectedItems().
You could ask a SeaMonkey person.

The Suite devs are not using Phab. The removals can go in or be removed from the patch. SeaMonkey in comm-central is currently broken and I can't test it. Pageinfo in suite needs to be aligned later with Fx. Thanks++

Comment 12

[Alex Vincent [:WeirdAl]]

checkin-needed: (1) I don't have checkin privileges to either repository. (2) These patches must land together. If one patch lands without the other, comm-central will break across the board.

Comment 13

[Alex Vincent [:WeirdAl]]

Also, if you need me for any support / changes, be advised I'm not available on Mondays and Wednesdays through the end of December 2019.

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: D39273.diff

I rebased the patch if you want to land it.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f58e37830b0
Remove performAction* from nsITreeView.idl in mozilla-central. r=peterv, johannh

Comment 16

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/0f58e37830b0

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/96c6aeb05665
Remove performAction* from nsITreeView.idl in comm-central. r=jorgk

Comment 18

[Ian Neal]

(In reply to Tim Nguyen :ntim from comment #0)

performActionOnRow appears to be only used once:
https://searchfox.org/mozilla-central/rev/
5117a4c4e29fcf80a627fecf899a62f117368abf/browser/base/content/pageinfo/
pageInfo.js#80-89

...and that usage actually seems broken. The copy context menu always
appears disabled and the keyboard shortcut also does not work.

performAction, performActionOnCell actually seem unused everywhere.

I haven't checked for comm-central.

Having just tested pageInfo with Firefox 68 on Linux, the copy context menu is not disabled and it does work. Unfortunately I cannot tell from the original report what version of Firefox the brokenness was against. Either way, as this has removed useful functionality, please can these changes be backed out?

Comment 19

[Alex Vincent [:WeirdAl]]

This landed in Firefox 70 just a couple days ago, and there are no plans to backport this to FF68 ESR. Can you please retest with a nightly build from this week?

My goal was very much not to regress anything, so if you found something that this regressed, please file a new bug and cc both myself and johannh.

Comment 20

[Alex Vincent [:WeirdAl]]

Re-resolving as this is for FF70, not FF68, and I want to handle regressions in a new bug.

Comment 21

[Ian Neal]

(In reply to Alex Vincent [:WeirdAl] from comment #19)

This landed in Firefox 70 just a couple days ago, and there are no plans to backport this to FF68 ESR. Can you please retest with a nightly build from this week?

My goal was very much not to regress anything, so if you found something that this regressed, please file a new bug and cc both myself and johannh.

My point is that copy in page info was not broken on Firefox 68, so there was no reason for the functionality to be removed in Firefox 70 but I have logged a new bug as requested.