Closed Bug 1508169 Opened Last year Closed 3 months ago

Remove performAction, performActionOnRow and performActionOnCell methods from tree interface

Categories

(Core :: XUL, task, P5)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: ntim, Assigned: WeirdAl)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 3 obsolete files)

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.
Priority: -- → P5
Type: enhancement → task

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.

Attached patch bug1508169-mozilla-central.diff (obsolete) — Splinter Review
Attached patch bug1508169-comm-central.diff (obsolete) — Splinter Review

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

Assignee: nobody → ajvincent

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 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.
Attachment #9076978 - Attachment is obsolete: true
Attached patch bug1508169-mozilla-central.diff (obsolete) — Splinter Review

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.

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.

Attachment #9076979 - Attachment is obsolete: true
Attachment #9077958 - Attachment is obsolete: true
Attachment #9080509 - Attachment description: Remove performAction* from nsITreeView.idl in mozilla-central. → Bug 1508169, Remove performAction* from nsITreeView.idl in mozilla-central. r=peterv
Attachment #9080509 - Attachment description: Bug 1508169, Remove performAction* from nsITreeView.idl in mozilla-central. r=peterv → Bug 1508169, Remove performAction* from nsITreeView.idl in mozilla-central. r=peterv, johannh

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++

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.

Keywords: checkin-needed

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.

Attached patch D39273.diffSplinter Review

I rebased the patch if you want to land it.

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

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/96c6aeb05665
Remove performAction* from nsITreeView.idl in comm-central. r=jorgk

(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?

Status: RESOLVED → REOPENED
Flags: needinfo?(ajvincent)
Resolution: FIXED → ---

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.

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

Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Flags: needinfo?(ajvincent)
Resolution: --- → FIXED
Blocks: 1577372
Regressions: 1577372

(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.

No longer blocks: 1577372
You need to log in before you can comment on or make changes to this bug.