Edgewall Software

Changes between Version 4 and Version 12 of Ticket #10994


Ignore:
Timestamp:
Feb 7, 2013, 6:19:48 AM (7 years ago)
Author:
Ryan J Ollos <ryan.j.ollos@…>
Comment:

Replying to cboos:

  1. I'm confused about where I should be placing the CDATA section tags.

We apparently do it only when needed (i.e. when using plain '<', '&' or '>' characters in the Javascript code), but I wouldn't object using it everywhere for consistency (and for not having to think about it!).

In the latest patch for #11056, I've included CDATA tags for almost all of the changed code in script elements. The one case I've found where this shouldn't be done is if there is embedded Genshi in the script, such as <py:if test="preview">...</py:if>. I guess that's obvious in retrospect since we need that Genshi to be parsed!

  1. I've used this pattern change(function() {...}).eq(0).change() (in this case click(function() {...}).eq(0).click() would probably be the same), rather than .click(someFunction); someFunction(); (i.e. passing a function to click and then calling that function directly). Using change seems a bit cleaner and both patterns are present throughout the Trac codebase.

I suppose it all depends on whether there's a side-effect or not, and if yes, if the side-effect it predictable, or not (e.g. what would happen if there would be other event handlers?). The second pattern ensure you have a strict control over what you force during this initial call.

The strict control sounds better for long-term maintainability. Thanks for giving it some thought and explaining the trade-off.

I'll put further comments on #11056, but just wanted to follow-up on this earlier discussion here.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #10994

    • Property Owner set to Christian Boos
    • Property Status newassigned
    • Property Milestonenext-dev-1.1.x
  • Ticket #10994 – Description

    v4 v12  
    66 * A ''Revert changes'' button is added. The button is disabled until a change is made.
    77 * ~~The ''Save changes'' button is disabled until a change is made.~~
    8  * The ''Remove selected items'' button is disabled until a checkbox is selected.
     8 * ~~The ''Remove selected items'' button is disabled until a checkbox is selected.~~ => #11056
    99 * Addition of a ''Select All'' checkbox in the table header.
    1010