Closed Bug 332945 Opened 18 years ago Closed 18 years ago

expose base interface binding for xhtml/xul calendars

Categories

(Core Graveyard :: XForms, defect)

Product:
Core Graveyard
Component:
XForms
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

(Keywords: fixed1.8.0.5, fixed1.8.1)

Attachments

(1 file, 2 obsolete files)

for checkin
20.51 KB, patch
Details | Diff | Splinter Review
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — — Splinter Review 

        Attachment #217384 -
        Flags: review?(doronr)

    
  

        Attachment #217384 -
        Flags: review?(allan)

    
    

    
  
A small note about what patch does. XUL and XHTML calendar can have a common base. Acutally the patch does it :). I move out some functionality from "calendar-compact" binding to new binding "calendar-compact-base".

    
    

    
  
Comment on attachment 217384 [details] [diff] [review]
patch

>diff -uNrap mozilla.orig/extensions/xforms/resources/content/widgets-xhtml.xml mozilla/extensions/xforms/resources/content/widgets-xhtml.xml
>--- mozilla.orig/extensions/xforms/resources/content/widgets-xhtml.xml	2006-04-05 12:32:25.000000000 +0900
>+++ mozilla/extensions/xforms/resources/content/widgets-xhtml.xml	2006-04-06 15:11:08.000000000 +0900
>@@ -41,18 +41,23 @@
>   %xformsDTD;
> ]>
> 
>+<!--
>+  The file contains auxiliary controls implementations for XHTML context. The
>+  controls are inherited from bindings defined in 'widgets.xml' file.
>+-->
> 
> <bindings id="widgetsBindingsForXHTML"
>           xmlns="http://www.mozilla.org/xbl"
>           xmlns:html="http://www.w3.org/1999/xhtml"
>-          xmlns:xbl="http://www.mozilla.org/xbl">
>+          xmlns:xbl="http://www.mozilla.org/xbl"
>+          xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">

If this is the xhtml impl, why the xul namespace?

>+          switch (aActionType) {
>+          case "prevMonth":
>+            this.setDate(this.year, this.month - 1, aDay);
>+            break;
>+
>+          case "nextMonth":
>+            this.setDate(this.year, this.month + 1, aDay);
>+            break;
>+
>+          case "currentMonth":
>+            if (!this.readonly) {
>+              this.selectedDate = new Date(this.year, this.month - 1, aDay);
>+              this.fireChangeEvent();
>+            } else {
>+              this.currentDayIndex = this.dayOffset + aDay - 1;
>+            }
>+            break;
>+          }

I prefer case statements indented, but that might just be me.

        Attachment #217384 -
        Flags: review?(doronr) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch2 (obsolete)
        — 
          — Splinter Review
      

    


  
(In reply to comment #3)

> If this is the xhtml impl, why the xul namespace?

Fixed.

> I prefer case statements indented, but that might just be me.
> 

Fixed.

        Attachment #217384 -
        Attachment is obsolete: true

        Attachment #221780 -
        Flags: review?(allan)

        Attachment #217384 -
        Flags: review?(allan)

    
    

    
  
Comment on attachment 221780 [details] [diff] [review]
patch2

> +  <!-- COMPACT CALENDAR BASE
> +    The widget is interface binding for xhtml and xul calendars widgets.

This widget is the interface binding ...

> +    It supposes successor widgets have following interface:

It assumes that successor widgets have ...

> +      <method name="setSelectedDayByIndex">
> +        <parameter name="aSelectedIndex"/>
> +        <parameter name="aCurrentIndex"/>
> +        <body>
> +        <![CDATA[
> +          if (!aCurrentIndex)
> +            aCurrentIndex = aSelectedIndex;
> +
> +          if (this._selectedDayIndex != -1) {
> +            this.unselectDayControl(this._dayElements[this._selectedDayIndex]);
> +          }
> +
> +          if (this.isSelectedDate()) {
> +            var dayElm = null;

You are not using this.

+            this.selectDayControl(this._dayElements[aSelectedIndex]);
+            this._selectedDayIndex = aSelectedIndex;
+          } else {
+            this._selectedDayIndex = -1;
+          }
+
+          this.currentDayIndex = aCurrentIndex;
+        ]]>
+        </body>
+      </method>

With that, r=me

        Attachment #221780 -
        Flags: review?(allan) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        for checkin
        — 
          — Splinter Review
      

    


  

        Attachment #221780 -
        Attachment is obsolete: true

    
    

    
  
Fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

    
  
Keywords: fixed1.8.1

    
  
Keywords: fixed1.8.0.5

    
  
Whiteboard: xf-to-branch
Product: Core → Core Graveyard

          You need to log in
          before you can comment on or make changes to this bug.