Questions:

1. Do we need to hide the root context div too if  
'css_hide_canvas_during_init'?

2. This

> this.quirks.fix_contextmenu && this.__LZcontextcontainerdiv


seems redundant, since you will only have a context containerdiv if  
the quirk is set.

3. Remind me again why we need a contextcontainerdiv _and_ a contextdiv?

4. Rather than unsetting clip by setting it to 'rect(auto auto auto  
auto)', why not just set it to ''?

5. If you are copying _all_ of the local styles from one div to  
another, the fastest way is to say:

> newdiv.style = olddiv.style.cssText

6. You should just remove this else clause:

>             cxdiv.id = 'lzcontextdiv';

because it 'is an error' to have more than one div with the same ID in  
a DOM (even if all browsers will let you get away with it).

7. Should setID also set the id on the context div if it exists?

8. Why do we need 'istextsprite'?  Can't you just say `foo instanceof  
LzTextSprite`?

Noticed in passing:

1. Is 'safari_visibility_instead_of_display' still necessary?  If it  
is, how come we only use it on the div and not the clickdiv?

On 2009-07-28, at 21:22EDT, Max Carlson wrote:

> Change 20090728-maxcarlson-L by maxcarl...@bank on 2009-07-28 17:53:48
> PDT
>     in /Users/maxcarlson/openlaszlo/trunk-clean
>     for http://svn.openlaszlo.org/openlaszlo/trunk
>
> Summary: Size views with no bgcolor or resource to 0x0, add separate
> tree for context menus
>
> Bugs Fixed: LPP-5447 - DHTML: inputtext and clickable (partial)
>
> Technical Reviewer: ptw
> QA Reviewer: [email protected],hminsky
>
> Details: LzSprite - (from ptw's change - Move the canvas hiding from
> the CSS class style to the canvas div, so removing it just removes the
> div style (and the div reverts to the class style default).  Similarly
> for controlling visibility on all divs.)  Add quirks property for
> sprite constructor.  Add fix_contextmenu and size_blank_to_zero
> quirks, default to on.  When fix_contextmenu quirk is on, build
> context menu container div called lzcanvascontextdiv that lives under
> the lzcanvasdiv and lzcanvasclickdiv.  Set x and y position,
> visibility, clipping and z-index of context container, if it exists.
> Lazily build up context menu div tree when context menu is set.  Base
> __LZclick div on cached width and height values.  When
> size_blank_to_zero quirk is on and there's no bgcolor or source
> property set (and we're not a textsprite), set the width/height to 0
> and set __sizedtozero flag so size can be restored as needed.  Set the
> context menu height/width if needed.  Restore div size when bgcolor or
> source is set to a non-null value.  Modify __findParents() to accept
> an optional second argument - when true, look for parents with a null
> value.  Change __updateClip() to update contextmenu and click
> container div clip values.  Clean up context menu and context menu
> container divs in destroy().  Cache value passed to setID() so it can
> be used later.
>
> LzTextSprite - Add istextsprite flag to test for text sprite classes
> more easily.
>
> LzMouseKernel - If fix_contextmenu quirk is on, hide visible and click
> div trees so context menu tree can be checked to find the correct
> context menu to show.
>
> Tests: Testcase attached to LPP-7661 works as before, as does the
> testcase from Maynard on 23/Feb/09 12:12 PM.  This change will make it
> possible for my recent changeset for LPP-5447 to work with context
> menus...
>
> Files:
> M      WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js
> M      WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js
> M      WEB-INF/lps/lfc/kernel/dhtml/LzMouseKernel.js
>
> Changeset: 
> http://svn.openlaszlo.org/openlaszlo/patches/20090728-maxcarlson-L.tar
> _______________________________________________
> Laszlo-reviews mailing list
> [email protected]
> http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews

_______________________________________________
Laszlo-reviews mailing list
[email protected]
http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews

Reply via email to