Closed Bug 1410713 Opened 3 years ago Closed 3 years ago

Port bug 1408572 to TB: Remove handleCtrlPageUpDown attribute and associated code

Categories

(Thunderbird :: Toolbars and Tabs, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 58.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(1 file, 2 obsolete files)

Bug 1408572 removed the handleCtrlPageUpDown property. It makes now no sense to let it in our tabmail.xml. Remove it like FX did.
This is a port of https://hg.mozilla.org/mozilla-central/rev/8e49aaf48550#l1.12
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8920874 - Flags: review?(jorgk)
Please include IB and SM in your patch. Looks like this code needs to go
https://dxr.mozilla.org/comm-central/rev/afdac93f14107db31e1f9d82b966b1b06e46d777/im/content/tabbrowser.xml#1401-1408
and line 27 in the same file needs adjustment as well as twos file in SM.
IB and SM added.

The development of IB stopped and I think we don't need no more look that it builds. TB will be the successor of IB, see http://blog.queze.net/post/2017/10/18/Thunderbird-is-the-next-version-of-Instantbird.
Attachment #8920874 - Attachment is obsolete: true
Attachment #8920874 - Flags: review?(jorgk)
Attachment #8920878 - Flags: review?(jorgk)
Comment on attachment 8920878 [details] [diff] [review]
No-handleCtrlPageUpDown-property.patch

Review of attachment 8920878 [details] [diff] [review]:
-----------------------------------------------------------------

You've got a syntax error there.

OK, well, if IB is no longer maintained, we should perhaps remove the code since I personally don't want to find deprecated features in the code base.

::: im/content/tabbrowser.xml
@@ +1397,5 @@
>  #else
>            if (('ctrlKey' in aEvent && aEvent.ctrlKey) &&
>                !('shiftKey' in aEvent && aEvent.shiftKey) &&
>                !('metaKey' in aEvent && aEvent.metaKey)) {
> +            if (aEvent.keyCode == KeyEvent.DOM_VK_F4 {

Closing parentheses missing here. Also, as handleCtrlPageUpDown got removed, the original condition will never be true any more so the entire block should be removed?

What is that doing anyway? Removing a tab when the user presses Ctrl+F4? Maybe that's still useful and you are right only removing handleCtrlPageUpDown from the condition.
Attached patch 22404.patchSplinter Review
(In reply to Jorg K (GMT+2) from comment #4)
> Comment on attachment 8920878 [details] [diff] [review]
> No-handleCtrlPageUpDown-property.patch
> 
> Review of attachment 8920878 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You've got a syntax error there.
> 
> OK, well, if IB is no longer maintained, we should perhaps remove the code
> since I personally don't want to find deprecated features in the code base.

They move now some code to TB. 

> ::: im/content/tabbrowser.xml
> @@ +1397,5 @@
> >  #else
> >            if (('ctrlKey' in aEvent && aEvent.ctrlKey) &&
> >                !('shiftKey' in aEvent && aEvent.shiftKey) &&
> >                !('metaKey' in aEvent && aEvent.metaKey)) {
> > +            if (aEvent.keyCode == KeyEvent.DOM_VK_F4 {
> 
> Closing parentheses missing here. Also, as handleCtrlPageUpDown got removed,
> the original condition will never be true any more so the entire block
> should be removed?

Fixed.

> What is that doing anyway? Removing a tab when the user presses Ctrl+F4?
> Maybe that's still useful and you are right only removing
> handleCtrlPageUpDown from the condition.

I don't know, you asked me to port it also for IB. It's hard to fix something we can't check.
Attachment #8920878 - Attachment is obsolete: true
Attachment #8920878 - Flags: review?(jorgk)
Attachment #8920900 - Flags: review?(jorgk)
Comment on attachment 8920900 [details] [diff] [review]
22404.patch

Thanks. What happened to the patch name? Is this the 22404th patch you're submitting ;-)

(In reply to Richard Marti (:Paenglab) from comment #5)
> I don't know, you asked me to port it also for IB. It's hard to fix
> something we can't check.
Agreed. But if we don't remove use of dead/deprecated M-C features, we will forever have them show up in our searches. That's why I filed bug 1410739.
Attachment #8920900 - Flags: review?(jorgk) → review+
No, it's the 22404th changeset in c-c. Actually there is one less as I have a other patch in front of this one. TortoiseHG proposed this name and it didn't got the correct name.
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/a400770aa527
Port bug 1408572 to C-C: Remove handleCtrlPageUpDown attribute and associated code. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
You need to log in before you can comment on or make changes to this bug.