211545 - [gtk] random string cleanup

Status: RESOLVED WONTFIX

(Core Graveyard :: GFX: Gtk, defect)

Description

[[e-mail]]

during alecf's recent charset whackage, I noticed some odd string fu in
widget/src/gtk. he promised to sr a little string cleanup at a later date ;)

Comment 1

[[e-mail]]

Attachment: patch v1

Comment 2

[[e-mail]]

Comment on attachment 126986 [details] [diff] [review]
patch v1

just a little string cleanup - feel free to take your time with it. thx!

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

*** Bug 211544 has been marked as a duplicate of this bug. ***

Comment 4

[Alec Flett]

Comment on attachment 126986 [details] [diff] [review]
patch v1

 nsClipboard::nsClipboard()
+ : mIgnoreEmptyNotification(PR_FALSE)
 {
 #ifdef DEBUG_CLIPBOARD
   g_print("nsClipboard::nsClipboard()\n");
 #endif /* DEBUG_CLIPBOARD */

-  mIgnoreEmptyNotification = PR_FALSE;
-  mGlobalTransferable = nsnull;
-  mSelectionTransferable = nsnull;
-  mGlobalOwner = nsnull;
-  mSelectionOwner = nsnull;
-  mSelectionData.data = nsnull;
   mSelectionData.length = 0;

we dont' need to init these variables? (just checking..)
sr=alecf with an explanation or the variables initialized

nice cleanup!

Comment 5

[[e-mail]]

good catch - looks like I was a bit overzealous... the transferables/owners are
COMPtrs so they don't need initing, but mSelectionData.data does. new patch
coming up...

Comment 6

[[e-mail]]

Attachment: patch v1.1

carrying over sr=alecf.

Comment 7

[[e-mail]]

Comment on attachment 127766 [details] [diff] [review]
patch v1.1

trying a different reviewer.

Comment 8

[Brian Ryner (not reading)]

Comment on attachment 127766 [details] [diff] [review]
patch v1.1

>Index: widget/src/gtk/nsClipboard.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/gtk/nsClipboard.cpp,v
>retrieving revision 1.112
>diff -u -1 -0 -p -r1.112 nsClipboard.cpp
>--- widget/src/gtk/nsClipboard.cpp	11 Jun 2003 18:15:33 -0000	1.112
>+++ widget/src/gtk/nsClipboard.cpp	15 Jul 2003 01:11:13 -0000
>@@ -535,28 +519,28 @@ nsClipboard::SelectionReceiver (GtkWidge
> #endif
>     mSelectionData.length = aSD->length;
>     return;
>   }
> 
>   char *str = gdk_atom_name(aSD->type);
>   nsCAutoString type(str);
>   g_free(str);
> 
> #ifdef DEBUG_CLIPBOARD
>-  g_print("        Type is %s\n", type.mBuffer);
>+  g_print("        Type is %s\n", type.get());
> 
>-  if (type.Equals("ATOM")) {
>+  if (type.Equals(NS_LITERAL_CSTRING("ATOM"))) {

Is there an advantage to this type of change? (you're doing it in several
places, and IMO it makes the code less readable)

>@@ -1276,58 +1239,40 @@ void nsClipboard::SetCutBuffer()
>     nsPrimitiveHelpers::ConvertUnicodeToPlatformPlainText (castedUnicode, dataLength / 2,
>                                                            &plainTextData, &plainTextLen);
>     if (clipboardData) {
>       nsMemory::Free(NS_REINTERPRET_CAST(char*, clipboardData));
>       clipboardData = plainTextData;
>       dataLength = plainTextLen;
>     }
>   }
> 
>   XRotateBuffers(GDK_DISPLAY(), 1);
>-  XStoreBytes(GDK_DISPLAY(), (const char *) clipboardData, nsCRT::strlen((const char *)clipboardData));
>+  XStoreBytes(GDK_DISPLAY(), (const char *) clipboardData, PL_strlen((const char *)clipboardData));

I think we can just use strlen() here.

Comment 9

[Brian Ryner (not reading)]

Comment on attachment 127766 [details] [diff] [review]
patch v1.1

marking review- to get this out of my queue... feel free to re-request review
if you've added explanations for the questions I had and still feel that this
patch is good.

Comment 10

[Mats Palmgren (inactive)]

widget/src/gtk/ has been removed on trunk.
This bug doesn't look like a branch candidate to me.

-> WONTFIX