211914 - Code Inspection: nsBoxFrame.cpp line 538 'NS_STYLE_BOX_ALIGN_STRETCH' not handled

Status: RESOLVED INVALID

(Core :: XUL, defect)

Description

[Ben Fowler]

User-Agent:       iCAB/2.9(Macintosh; U; PPC; compatible; Mozilla/7.01; MSIE 8.0)
Build Identifier: 

The switch statement does not handle the constant
NS_STYLE_BOX_ALIGN_STRETCH.

It is possible that this code (which seems to have been
written a long time ago) ought to handle STRETCH as a
sub-case of START.

It is also possible that since the macro
NS_STYLE_BOX_ALIGN_STRETCH has the value 0, that when this
value, O is being used, the BoxInfo concerned is not fully
constructed, and some default setting (perhaps STRETCH or
START in fact) is appropriate.

It is difficult to suggest a fix, but I would be surprised
if there were not a problem of some kind here.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.




Apologies if Iam treading on anyone's toes by publishing this. It is possible
that I have simply misunderstood the code, plus I don't see a 'severity' for simply
raising a query on 'code quality' of the source. Should we bw doing this at al???

Comment 1

[Ben Fowler]

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

So.. does anyone other than hyatt actually know this code?  ;)

Comment 3

[Jasper]

please update

Comment 4

[Bruce Davidson]

Not clear why this bug is raised against Camino as its not in Camino (or even
Mac)-specific code. Changing to Browser product.

Comment 5

[[e-mail]]

Stretching is handled in nsBoxFrame::GetInitialAutoStretch

Comment 6

[[e-mail]]

I supposed I'd better explain, for the record.
Imagine you have a parent box with two child boxes. The parent has a size of
40x40 while the child boxes have preferred sizes of 20x20 and 10x10.
The old box model code used orient, align, valign and autostretch styles.
Normally the parent would stretch its children in the opposite direction to its
orient, so a box with vertical orient would stretch its child boxes to 40x20 and
40x10. You could turn this off with autostretch="never" at which point the left,
center or right alignment would come in to play. For a box with horizontal
orient the child boxes would be stretched to 20x40 and 10x40, so you would need
to use autostretch="never" to allow you to vertically align the child boxes to
top, middle or bottom.
The new box model code uses orient, align and pack styles. In this case, align
always works in the opposite direction to the orient, so setting an align
automatically turns off stretch.

Comment 7

[Ben Fowler]

O.K. IIRC, there is a compiler warning for the switch not handling
the value NS_STYLE_BOX_ALIGN_STRETCH (and if not, perhaps there should be!).

This patch should make things clear.

@@ -529,7 +544,11 @@
       case NS_STYLE_BOX_ALIGN_END:
         aValign = nsBoxFrame::vAlign_Bottom;
         return PR_TRUE;
+      case NS_STYLE_BOX_ALIGN_STRETCH:
+        // Handled in GetInitialAutoStretch( )
+        return PR_FALSE;
       default: // Nonsensical value. Just bail.
+        NS_NOTREACHED( "nsBoxFrame::GetInitialVAlignment Attribute had
nonsensical value" );
         return PR_FALSE;
     }
   }

If the comment about nonsensical value on the default case is correct,
then perhaps the code should agree with it ...