70704 - [XUL Syntax] Add XUL display types to CSS

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[David Hyatt]

The basic XUL display types should be in CSS.

Comment 1

[David Hyatt]

Attachment: Patch to content DLL

Comment 2

[David Hyatt]

Attachment: Patch to layout DLL

Comment 3

[David Hyatt]

Looking for review, r/sr from attinasi, pierre, brendan.  This is the first of 
several CSS patches that I'll be producing as visual properties from XUL move 
into CSS where they belong.

All of the display types are properly prefaced with -moz-.

Comment 4

[Marc Attinasi]

I like where this is going. Do we want to #ifdef XUL all of the constants,
keywords and properties as well as the code that uses it? It is unclear to me if
this is a good idea or not: it would allow reduced memory footprint if XUL is
not defined (admitedly small), but it would be, uh, messy. David, your thoughts?

Comment 5

[David Hyatt]

My thought was that it wasn't worth cluttering up the code with #ifdefs in
places where there's no real code/footprint impact, e.g., the definition of
constants and keywords.

Comment 6

[Marc Attinasi]

I guess I have to agree - clutter is bad and the memory required is tiny.
[s]r=attinasi

Comment 7

[Peter Trudelle]

mass-targetting to mozilla1.0, adding helpwanted keyword

Comment 8

[David Hyatt]

Another reason not to #ifdef the keywords and constants is that these UI
elements have been proposed to XForms, and Ian is writing them up for CSS3, so
they may not even stay XUL for long anyway. :)

Anyone else on the cc list want to give me an r=?

Comment 9

[Brendan Eich [:brendan]]

Pierre's right, we shouldn't duplicate those keyword and property lists.  I see
another bug about fixing that, so on to the nit-picking!

Select the following, notice the added (+) line of spaces -- I hate when that
happens:

@@ -5253,8 +5273,7 @@
       // its default orientation is horizontal for hbox and vertical for vbox
       rv = NS_NewBoxFrame(aPresShell, &newFrame, PR_FALSE, nsnull, aTag !=
nsXULAtoms::vbox);
 
-      const nsStyleDisplay* display = (const nsStyleDisplay*)
-           aStyleContext->GetStyleData(eStyleStruct_Display);
+      
 
       // Boxes can scroll.
       if (IsScrollable(aPresContext, display)) {

Here, the final line of the condition is indented one space too many:

+    else if ((!aXBLBaseTag && (display->mDisplay == NS_STYLE_DISPLAY_INLINE_GRID ||
+                               display->mDisplay == NS_STYLE_DISPLAY_GRID)) ||
+              aTag == nsXULAtoms::grid || aTag == nsXULAtoms::tree) {

Otherwise, looks good -- fix these if you share my anal-retentive appreciation
for indentation, and rs=[e-mail].

/be

Comment 10

[David Hyatt]

Fixed.

Comment 11

[David Hyatt]

Reopening.  But wait, there's more! :)

Comment 12

[David Hyatt]

Ok, this next patch adds all the remaining XUL widgets that merit their own 
display types.  (I view all other XUL widgets as being one of these display 
types with some additional associated behavior.)

Comment 13

[David Hyatt]

Attachment: Patch #2 to content DLL

Comment 14

[David Hyatt]

In the #2 content DLL patch, ignore all the stuff about scoped roots.  That's a 
patch for another bug.  Just look at the addition of the keywords.  That's all 
that's relevant for this bug.

Comment 15

[David Hyatt]

Attachment: Patch #2 to layout DLL

Comment 16

[David Hyatt]

Attachment: Patch #2 to xul.css

Comment 17

[David Hyatt]

Well, in this case, diff -u produces some pretty darn unreadable diffs.  Oh 
well.  All I'm doing in the layout patch is moving the frame construction code 
down to the end of the function and making it key off display type rather than 
tag for those elements that now have display types.  None of the code inside 
the ifs is changed.

Ready for an r from pierre for the style system portions, and sr from brendan 
or attinasi.

Comment 18

[Pierre Saslawsky]

The changes are fine but please rename the xul properties as "-moz-xul".
r=pierre

Comment 19

[Marc Attinasi]

sr=attinasi (assuming the changes to lazily initialize the mStyleRuleSupplier
 member of the SelectorMatchesData like in the other bug) Rock On, Dave!

Comment 20

[David Hyatt]

Fixed.