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

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, enhancement)

Description

[Richard Marti (:Paenglab)]

Bug 1408572 removed the handleCtrlPageUpDown property. It makes now no sense to let it in our tabmail.xml. Remove it like FX did.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: No-handleCtrlPageUpDown-property.patch

This is a port of https://hg.mozilla.org/mozilla-central/rev/8e49aaf48550#l1.12

Comment 2

[Jorg K (CEST = GMT+2)]

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.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: No-handleCtrlPageUpDown-property.patch

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.

Comment 4

[Jorg K (CEST = GMT+2)]

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.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 22404.patch

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

Comment 6

[Jorg K (CEST = GMT+2)]

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.

Comment 7

[Richard Marti (:Paenglab)]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a400770aa527
Port bug 1408572 to C-C: Remove handleCtrlPageUpDown attribute and associated code. r=jorgk