Closed Bug 783715 Opened 12 years ago Closed 12 years ago

Switch to new event constructors

Categories

(SeaMonkey :: General, enhancement)

Product:
SeaMonkey
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
seamonkey2.17

People

(Reporter: neil, Assigned: ewong)

References

Details

(Keywords: helpwanted, Whiteboard: [good first bug][lang=js][mentor=Neil])

Attachments

(2 files, 2 obsolete files)

Switch to new event constructors (v3)
12.53 KB, patch
neil
: review+
Details | Diff | Splinter Review
Trailing whitespace fix.
1.91 KB, patch
ewong
: review+
Details | Diff | Splinter Review 
The traditional way of creating events was as follows:
var event = document.createEvent("CustomEvent");
event.initSomeEvent("media-showStatistics", aBubbles, aCancelable, aDetail);
element.dispatchEvent(event);

The new HTML5 way is as follows:
element.dispatchEvent(new CustomEvent("media-showStatistics",
  { bubbles: aBubbles, cancelable: aCancelable, detail: aDetail }));

Note that the number of parameters depends on the event type. See initXXXEvent and XXXEventInit in nsIDOMXXXEvent.idl for the list.
Whiteboard: [lang=js][mentor=Neil] → [good first bug][lang=js][mentor=Neil]
Assignee: nobody → ewong
Status: NEW → ASSIGNED

        Attachment #686478 -
        Flags: review?(neil)

    
    

    
  
Comment on attachment 686478 [details] [diff] [review]
Switch to new even constructors (v1)

Review of attachment 686478 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/browser/test/browser/browser_pluginplaypreview.js
@@ +209,5 @@
>    ok(rect.width == 200, "Test 1a, Plugin with id=" + plugin.id + " overlay rect should have 200px width before being replaced by actual plugin");
>    ok(rect.height == 200, "Test 1a, Plugin with id=" + plugin.id + " overlay rect should have 200px height before being replaced by actual plugin");
>  
> +  overlay.dispatchEvent(new CustomEvent("MozPlayPlugin", 
> +    { bubbles: true, cancelable: true, detail: null}));

Trailing whitespace

@@ +290,5 @@
>  
>    var overlay = doc.getAnonymousElementByAttribute(plugin, "class", "previewPluginContent");
>    ok(overlay, "Test 5a, the overlay div is expected");
>  
> +  overlay.dispatchEvent(new CustomEvent("MozPlayPlugin", 

And here

::: suite/common/tests/browser/browser_447951.js
@@ +26,5 @@
>        is(tabState.entries[0].url, baseURL + 0, "... but not more");
>        
>        // visit yet another anchor (appending it to session history)
>        let doc = tab.linkedBrowser.contentDocument;
> +      let event = new MouseEvent("click", 

and here

@@ +30,5 @@
> +      let event = new MouseEvent("click", 
> +        { bubbles: true, cancelable: true, view: doc.defaultView,
> +          detail: 1, screenX: 0, screenY: 0, clientX: 0, clientY: 0,
> +          ctrlKey: false, altKey: false, shiftKey: false, metaKey: false,
> +          button: 0, relatedTarget: null });

You probably don't need to add all those

::: suite/common/tests/browser/browser_522545.js
@@ +204,5 @@
>        is(browser.userTypedValue, null, "userTypedValue is empty to start");
>        is(browser.userTypedClear, 0, "userTypedClear is 0 to start");
>  
>        gURLBar.value = "example.org";
> +      gURLBar.dispatchEvent(new Event("input", 

Again

::: suite/mailnews/mailWidgets.xml
@@ +114,5 @@
>  
>        <method name="_fireOnSelect">
>          <body><![CDATA[
>            if (!this._suppressOnSelect && !this.suppressOnSelect) {
> +            this.dispatchEvent(new Event("select", 

.

::: suite/mailnews/msgHdrViewOverlay.js
@@ +250,5 @@
>    if (initialCollapsedSetting == "true")
>      gCollapsedHeaderViewMode = true;   
>  
>    // dispatch an event letting any listeners know that we have loaded the message pane
> +  toggleHeaderView.dispatchEvent(new Event('messagepane-loaded', 

.

::: suite/mailnews/tabmail.js
@@ +836,5 @@
>      gDBView.loadMessageByUrl("about:blank");
>      gDBView.selectionChanged();
>    }
> +
> +  var event = new Event( "messagepane-"+ (hidden ? "hide" : "unhide"),

Space before +, but not after (

    
    

    
  
(In reply to :Ms2ger from comment #2)
> @@ +30,5 @@
> > +      let event = new MouseEvent("click", 
> > +        { bubbles: true, cancelable: true, view: doc.defaultView,
> > +          detail: 1, screenX: 0, screenY: 0, clientX: 0, clientY: 0,
> > +          ctrlKey: false, altKey: false, shiftKey: false, metaKey: false,
> > +          button: 0, relatedTarget: null });
> 
> You probably don't need to add all those
> 

How do I determine which fields are needed?  I've learnt that 
bubbles, cancelable, view and detail are the ones that are needed
for MouseEvent.  The rest are new to me (though I cannot conclude
they shouldn't be included).

    
    

    
  
(In reply to Edmund Wong from comment #3)
> (In reply to Ms2ger from comment #2)
> > (From update of attachment 686478 [details] [diff] [review])
> > > @@ +30,5 @@
> > > +      let event = new MouseEvent("click", 
> > > +        { bubbles: true, cancelable: true, view: doc.defaultView,
> > > +          detail: 1, screenX: 0, screenY: 0, clientX: 0, clientY: 0,
> > > +          ctrlKey: false, altKey: false, shiftKey: false, metaKey: false,
> > > +          button: 0, relatedTarget: null });
> > You probably don't need to add all those
> How do I determine which fields are needed?
As far as I can tell, you only need the fields which differ from the default value. Most dictionaries don't override the default default values of false, zero or null as appropriate (ArchiveReader and CameraSelector are the only two that do and they aren't even event types).

    
    

    
  
Comment on attachment 686478 [details] [diff] [review]
Switch to new even constructors (v1)

>diff --git a/suite/browser/test/browser/browser_pluginplaypreview.js b/suite/browser/test/browser/browser_pluginplaypreview.js
>--- a/suite/browser/test/browser/browser_pluginplaypreview.js
>+++ b/suite/browser/test/browser/browser_pluginplaypreview.js
Probably best to avoid ported tests as it makes it harder to keep them in sync.

>diff --git a/suite/common/src/nsSessionStore.js b/suite/common/src/nsSessionStore.js
>--- a/suite/common/src/nsSessionStore.js
>+++ b/suite/common/src/nsSessionStore.js
Does new Event actually work here?

        Attachment #686478 -
        Flags: review?(neil) → feedback+

    
    

    
  


  

        Attachment #686478 -
        Attachment is obsolete: true

        Attachment #690069 -
        Flags: review?(neil)

    
    

    
  
Comment on attachment 690069 [details] [diff] [review]
Switch to new event constructors (v2)

>+            t.dispatchEvent(new Event("TabOpen",
>+              { bubbles: true, cancelable: false}));
Nit: space before }

>+            aTab.dispatchEvent(new UIEvent("TabClose",
>+              { bubbles: true, cancelable: false, view: window,
>+                detail: !!aParams.disableUndo}));
Nit: space before }

>+            tab.dispatchEvent(new UIEvent("TabMove",
>+              { bubbles: true, cancelable: false, view: window,
>+                detail: aSrcIndex}));
Nit: space before }

>diff --git a/suite/common/src/nsSessionStore.js b/suite/common/src/nsSessionStore.js
These changes definitely don't work, my session won't restore with the patch applied. r=me with them removed.

>diff --git a/suite/common/tests/browser/*
These are all ported tests, I'm afraid, so you'll have to remove these changes too. Maybe you can persuade Firefox to update tests to use new event constructors, and then you can port the changes over.

>+      aTextField.dispatchEvent(new UIEvent("input",
>+        { bubbles: true, cancelable: true,
>+          view: aTextField.ownerDocument.defaultView,
>+          detail: 0 }));
[Nit: don't need to provide detail here.]

>+      gURLBar.dispatchEvent(new Event("input", 
>+        { bubbles: true, cancelable: false }));
[Eek! This test is wrong, input is a UIEvent...]

        Attachment #690069 -
        Flags: review?(neil) → review+

    
    

    
  


  

        Attachment #690069 -
        Attachment is obsolete: true

        Attachment #690859 -
        Flags: review?(neil)

    
  

        Attachment #690859 -
        Flags: review?(neil) → review+

    
    

    
  
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/48e23cbddfa8
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.17

    
    

    
  
I'm getting these error messages:
Wed Dec 19 2012 22:05:38
Error: An error occurred updating the cmd_delete command: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgDBView.hdrForFirstSelectedMessage]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://messenger/content/folderDisplay.js :: gFolderDisplay.selectedMessage :: line 18"  data: no]
Source file: chrome://global/content/globalOverlay.js
Line: 81
 ----------
Wed Dec 19 2012 22:05:38
Error: An error occurred updating the button_junk command: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgDBView.hdrForFirstSelectedMessage]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://messenger/content/folderDisplay.js :: gFolderDisplay.selectedMessage :: line 18"  data: no]
Source file: chrome://global/content/globalOverlay.js
Line: 81
 ----------
Wed Dec 19 2012 22:05:38
Error: An error occurred updating the button_delete command: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgDBView.hdrForFirstSelectedMessage]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://messenger/content/folderDisplay.js :: gFolderDisplay.selectedMessage :: line 18"  data: no]
Source file: chrome://global/content/globalOverlay.js
Line: 81

Backing out this patch fixes the errors.

Possibly this bit is to blame:
http://hg.mozilla.org/comm-central/rev/48e23cbddfa8#l6.12
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
  
Status: REOPENED → ASSIGNED

    
    

    
  
So it turns out the problem is somewhere else. Re-closing. Sorry for the bug spam.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED

    
    

    
  


  

        Attachment #695075 -
        Flags: review+

    
    

    
  
Hmm, did we overlook the 5 events in MsgComposeCommands.js?
Blocks: 1099068

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: