[sldev] comments on patch for VWR-423

[Nicholaz Beresford]

Ham

I was looking over the patch (just the patch, didn't apply it)
but it somehow seems like it tries to fix things on both ends.

I'm not sure which approach I'd be choosing but I usually go for
minimum source impact and minimum side effect, so I'd probably
just do the isTextChanged trick.  But as said, I have not looked
at the specifics.


Also, there are a couple of lines in the patch with trailing
spaces or tabs which make the patch a bit harder to read than
necessary.

(1st hunk)
-	mWText = utf8str_to_wstring(mUTF8Text);
+	mWText = utf8str_to_wstring(mUTF8Text);	

(2nd hunk)
-	setText(value.asString());
+	setText(value.asString());	


(3rd hunk odd curlys)

-	if (mCommitOnFocusLost)
+	if (!mReadOnly && mCommitOnFocusLost && isTextChanged())		
  	{
-		onCommit();
+		{
+			onCommit();
+		}		
  	}

and again trailing tabs in texteditor.h @@ -340,7 +341,8 @@
-	mutable LLString mUTF8Text;
+	LLWString		mWPrev;
+	mutable LLString mUTF8Text;	


Nick


ham wrote:
> Hi all. I've attached a patch to VWR-423 to cover both VWR-423 and 
> VWR-1187.
> I've tested on a windows build using 1.18, but would like some more eyes on
> it since this is my first patch. VWR-423 is "Selecting group charter text
> causes Apply/Ignore/Cancel popup even if the text wasn't changed" and
> VWR-1187 is "Profile > Classifieds tab shows confirmation dialog when no
> changes are made"
> 
> http://jira.secondlife.com/browse/VWR-423
> http://jira.secondlife.com/browse/VWR-1187
> 
> There are two types of fixes here. For the first change LLTextEditor now
> only calls onCommit within LLTextEditor::onFocusLost if the text has 
> changed
> (similar to how LLLineEditor handle it).
> 
> The other changes are to filter calls to onFocusReceived in
> LLPanelClassified, LLPanelGroupRolesSubTab and LLPanelGroupGeneral. 
> There is
> an assumption made in the code for LLUICtrl::setFocus that disabled
> (readonly) controls never get the focus and hence never will call
> onFocusReceived.
> 
> You cannot disable the LLTextEditor when setting read only because that
> would take away the ability for users to scroll long text (such as no copy
> notecard). So all that to say...that the onFocusReceived and some onCommit
> calls had to filtered to make sure if onCommit is being called that the 
> user
> has the rights to change and that in fact the text is changed.
> 
> By itself, this fix won't affect performance of one viewer but I would
> imagine with all the update messages not happening, this change will help
> overall scale. One example this will help is that spurious PickInfoUpdate
> messages that were being generated just by clicking the description of
> someone else's Picks and losing focus.
> 
> Anywho...let me know if you have any problems or questions if you
> incorporate this patch.
> 
> thanks,
> 
> Jon Price/Hamncheese Omlet
> _______________________________________________
> Click here to unsubscribe or manage your list subscription:
> /index.html