Another CSS Selector bug?

[dave]

I've found what looks like another CSS selector bug. The selector is
appending the attributes of the input element to the attributes of the
root output element. This is especially problematic when on of those
attributes is id :-)

This is with lift-2.4-M4 and scala-2.9.1. From a console:

scala> import net.liftweb.util.Helpers._
import net.liftweb.util.Helpers._

scala> val input = <div id="foo">content</div>
input: scala.xml.Elem = <div id="foo">content</div>

scala> val selector =
| "#foo" #> ((ns: xml.NodeSeq) => <wrapper>{ns}</wrapper>)
selector: net.liftweb.util.CssSel = // snip

scala> println(selector(input))
<wrapper id="foo"><div id="foo">content</div></wrapper>

You can see that wrapper is mistakenly getting an id attribute. The
final output should be <wrapper><div id="foo">content</div></wrapper>

Thanks!
dave
<><

[dave]

On Sep 14, 8:43 am, dave <leedm...@yahoo.com> wrote:
> You can see that wrapper is mistakenly getting an id attribute.  The
> final output should be <wrapper><div id="foo">content</div></wrapper>

Hm. Looks like this is not new behavior for lift-2.4, but was
actually the way things worked in lift-2.3.

I still say that this is unexpected behavior, and is a bug that should
be fixed. Thoughts?

For those interested: the reason why I never noticed this behavior in
lift-2.3, is that in my actual code instead of <wrapper> I was using
<lift:surround>. In lift-2.3, lift:surround drops any extra
attributes passed to it. In lift-2.4, it appears to apply those
attributes to the root element of the surrounding template. Putting a
lift:children around the lift:surround in my snippet drops those pesky
extra attributes.

dave
<><

[Peter Brant]

Sounds reasonable. Please add a ticket and assign it to me. I'll
take care of this and the couple of other CSS transform tickets in the
next couple of days.

Pete

> --
> You received this message because you are subscribed to the Google Groups "Lift" group.
> To post to this group, send email to lif...@googlegroups.com.
> To unsubscribe from this group, send email to liftweb+u...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/liftweb?hl=en.
>
>

[dave]

On Sep 14, 1:34 pm, Peter Brant <peter.br...@gmail.com> wrote:
> Sounds reasonable.  Please add a ticket and assign it to me.  I'll
> take care of this and the couple of other CSS transform tickets in the
> next couple of days.
>
> Pete

Done - https://www.assembla.com/spaces/liftweb/tickets/1114

dave
<><

[David Pollak]

On Wed, Sep 14, 2011 at 7:34 PM, Peter Brant <peter...@gmail.com> wrote:

Sounds reasonable.  Please add a ticket and assign it to me.  I'll
take care of this and the couple of other CSS transform tickets in the
next couple of days.

Actually, I think attribute merging is feature, not a bug:

scala> val f = "*" #> <i/>
f: net.liftweb.util.CssSel = CssBind(Full(*), Full(StarSelector(Empty)))

scala> f(<a href="foo"/>)
res0: scala.xml.NodeSeq = NodeSeq(<i href="foo"></i>)

scala>

I spent a lot of time making it work correctly.

I'm all for a flag that says "don't merge attributes", but removing the feature would be a bad thing(tm)
 

--
Lift, the simply functional web framework http://liftweb.net

Simply Lift http://simply.liftweb.net
Follow me: http://twitter.com/dpp
Blog: http://goodstuff.im

[Peter Brant]

Ah, interesting. I don't think I've used that before (although there
are plenty of parts of the CSS selector syntax/behavior I haven't
used). What's the use case?

Pete

[dave]

On Sep 14, 3:29 pm, David Pollak <feeder.of.the.be...@gmail.com>
wrote:

> On Wed, Sep 14, 2011 at 7:34 PM, Peter Brant <peter.br...@gmail.com> wrote:
> > Sounds reasonable.  Please add a ticket and assign it to me.  I'll
> > take care of this and the couple of other CSS transform tickets in the
> > next couple of days.
>
> Actually, I think attribute merging is feature, not a bug:
>
> scala> val f = "*" #> <i/>
> f: net.liftweb.util.CssSel = CssBind(Full(*), Full(StarSelector(Empty)))
>
> scala> f(<a href="foo"/>)
> res0: scala.xml.NodeSeq = NodeSeq(<i href="foo"></i>)
>
> scala>
>
> I spent a lot of time making it work correctly.
>
> I'm all for a flag that says "don't merge attributes", but removing the
> feature would be a bad thing(tm)

Hm. I see where that can be useful, but it seems non-intuitive as the
default option. Then again, intuitiveness is in the eye of the
beholder. It's probably the case that attribute merging makes more
sense when the right hand side is a NodeSeq, and attribute replacement
makes more sense when the right hand side is a NodeSeq => NodeSeq.

Would the flag be a new replacement rule? If so, I propose:

% replaces all matching elements with the values, merging attributes
"* %" #> <i/> // <a href="foo"/> -> <i href="foo"/>

!% replaces all matching element, no attribute merging
"* !%" #> (ns: NodeSeq) => <i>{ns}</i> // <a href="foo"/> -> <i><a
href="foo"/></i>

% is equivalent to what currently happens when there is no replacement
rule.

And if anyone wants to change the default to !%, I wouldn't object :-)

dave
<><

[David Pollak]

On Wed, Sep 14, 2011 at 10:15 PM, dave <leed...@yahoo.com> wrote:

On Sep 14, 3:29 pm, David Pollak <feeder.of.the.be...@gmail.com>
wrote:

> On Wed, Sep 14, 2011 at 7:34 PM, Peter Brant <peter.br...@gmail.com> wrote:
> > Sounds reasonable.  Please add a ticket and assign it to me.  I'll
> > take care of this and the couple of other CSS transform tickets in the
> > next couple of days.
>
> Actually, I think attribute merging is feature, not a bug:
>
> scala> val f = "*" #> <i/>
> f: net.liftweb.util.CssSel = CssBind(Full(*), Full(StarSelector(Empty)))
>
> scala> f(<a href="foo"/>)
> res0: scala.xml.NodeSeq = NodeSeq(<i href="foo"></i>)
>
> scala>
>
> I spent a lot of time making it work correctly.
>
> I'm all for a flag that says "don't merge attributes", but removing the
> feature would be a bad thing(tm)

Hm.  I see where that can be useful, but it seems non-intuitive as the
default option.

One of the biggest complains about Helpers.bind() was that it did *not* merge attributes.  Basically, designers would do something like <input name="firstname" class="foo bar" id="baz"> and then in the CSS Selector Transforms, you do: "name=firstname" #> SHtml.text(name, name = _) and you *want* the attributes merged.


 

 Then again, intuitiveness is in the eye of the
beholder.  It's probably the case that attribute merging makes more
sense when the right hand side is a NodeSeq, and attribute replacement
makes more sense when the right hand side is a NodeSeq => NodeSeq.

Would the flag be a new replacement rule?  If so, I propose:

% replaces all matching elements with the values, merging attributes
 "* %" #> <i/> // <a href="foo"/> -> <i href="foo"/>

!% replaces all matching element, no attribute merging
  "* !%" #> (ns: NodeSeq) => <i>{ns}</i> // <a href="foo"/> -> <i><a
href="foo"/></i>

% is equivalent to what currently happens when there is no replacement
rule.

And if anyone wants to change the default to !%, I wouldn't object :-)

I'm not keen on the syntax... anyone got any other suggestions?
 

dave
<><

--
You received this message because you are subscribed to the Google Groups "Lift" group.
To post to this group, send email to lif...@googlegroups.com.
To unsubscribe from this group, send email to liftweb+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/liftweb?hl=en.

--

[Peter Brant]

On Thu, Sep 15, 2011 at 9:36 PM, David Pollak
<feeder.of...@gmail.com> wrote:

>
> One of the biggest complains about Helpers.bind() was that it did *not*
> merge attributes.  Basically, designers would do something like <input
> name="firstname" class="foo bar" id="baz"> and then in the CSS Selector
> Transforms, you do: "name=firstname" #> SHtml.text(name, name = _) and you
> *want* the attributes merged.
>

Yeah, very slick.

Pete