Re: [cp-patches] The latest two patches on javax/swing/plaf/basic/BasicListUI.java break the List World demo: One line should be reverted

2006-01-30 Thread Roman Kennke
Hi Audrius,

Am Montag, den 30.01.2006, 09:33 +0100 schrieb Audrius Meskauskas:
> The new JList component does not appear because maybeUpdateLayoutState 
> does not update the layout state if the list.isValid() returns false.
> 
> I would suggest avoid applying patches, breaking some already existing 
> functionality. Even if such patches are correct by themselves, the 
> methods that have used the incorrect behavior should be fixed in the 
> same or immediately followed path.

I fixed this issue. Honestly, I would suggest to not quickly reverting
patches that don't cause heavy breakage, where I do not consider this
particular bug as 'heavy'. Swing is a fast moving target and there is
the occasional bug popping up while working on other stuff. Reverting
other's work is not really productive. Time would be better spent by
filing bugreports and writing Mauve tests. Have a little patience.
Normally I am quick with fixing my fixes ;-) This is of course a
different issue, when we are short before a release, which we aren't.

Cheers, Roman



signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil


Re: [cp-patches] The latest two patches on javax/swing/plaf/basic/BasicListUI.java break the List World demo: One line should be reverted

2006-01-30 Thread Roman Kennke
Hi Audrius,

Am Montag, den 30.01.2006, 09:33 +0100 schrieb Audrius Meskauskas:
> The new JList component does not appear because maybeUpdateLayoutState 
> does not update the layout state if the list.isValid() returns false.
> 
> I would suggest avoid applying patches, breaking some already existing 
> functionality. Even if such patches are correct by themselves, the 
> methods that have used the incorrect behavior should be fixed in the 
> same or immediately followed path.

Sure. If I had noticed the regression I would have fixed it before
committing this stuff. I'm going to fix this today.

/Roman



signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil


Re: [cp-patches] The latest two patches on javax/swing/plaf/basic/BasicListUI.java break the List World demo: One line should be reverted

2006-01-30 Thread Audrius Meskauskas
The new JList component does not appear because maybeUpdateLayoutState 
does not update the layout state if the list.isValid() returns false.


I would suggest avoid applying patches, breaking some already existing 
functionality. Even if such patches are correct by themselves, the 
methods that have used the incorrect behavior should be fixed in the 
same or immediately followed path.


2006-01-28  Audrius Meskauskas  <[EMAIL PROTECTED]>

  * javax/swing/plaf/basic/BasicListUI.java  (maybeUpdateLayoutState):  
Consider the validation state of

   the list.

Audrius.

Index: BasicListUI.java
===
RCS file: /sources/classpath/classpath/javax/swing/plaf/basic/BasicListUI.java,v
retrieving revision 1.52
diff -u -r1.52 BasicListUI.java
--- BasicListUI.java	28 Jan 2006 20:34:00 -	1.52
+++ BasicListUI.java	30 Jan 2006 08:19:12 -
@@ -899,7 +899,7 @@
*/
   protected void maybeUpdateLayoutState()
   {
-if (updateLayoutStateNeeded != 0)
+if (updateLayoutStateNeeded != 0 || !list.isValid())
   {
 updateLayoutState();
 updateLayoutStateNeeded = 0;


[cp-patches] The latest two patches on javax/swing/plaf/basic/BasicListUI.java break the List World demo

2006-01-29 Thread Audrius Meskauskas

Hi, Roman,

the latest two patches on BasicListUI break the list word demo in the 
Swing activity board (adding new element (by pressing the button) no 
longer adds the elements to the end of the list.


It is possible that:
1. We may need to revert the patches or
2. Probably patches are correct and something inside the JList must be 
fixed instead.


For you, it may be easier to check which case is correct. If the JList 
works differently than expected, we should fill in the bug report.


2006-01-28  Roman Kennke  <[EMAIL PROTECTED]>

   * javax/swing/plaf/basic/BasicListUI.java
   (updateLayoutState): Removed unneeded special case for VERTICAL.

2006-01-28  Roman Kennke  <[EMAIL PROTECTED]>

   * javax/swing/plaf/basic/BasicListUI.java
   (getCellBounds): Determine correct list width when having a
   layoutOrientation of VERTICAL.
   (maybeUpdateLayoutState): Don't consider the validation state of
   the list.