Event extraction behavior in JsExps that reference HTML

[Antonio Salazar Cardozo]

Hey folks,

Back in June, Andreas found an issue with the interactions between Lift 3's event

extraction feature and JsExp expressions that referenced HTML (like, for example,

the JQuery JqAppend helper). Specifically in these cases, the generated code would

run the event handler attachment JavaScript when the HTML was passed to the JS,

which means it was being executed before the HTML was actually attached to the

DOM.

In a discussion on a PR for fixing the issue, we realized that the problem was more

complicated than originally understood. Andreas wanted a particular behavior (event

attachment occurring after the DOM insertion, but before any other chained JQuery

updates were handled),  and general reworking would be required no matter what

our updated approach is.

With all that background out of the way, there's a decision facing us now:

 - We keep the current behavior, which will try to run event handler attachment before

   DOM insertion. This is obviously broken.

 - We don't do event handler extraction in JsExps, even if extractInlineJavaScript is

   enabled.

 - We update event handler extraction for JsExps so that any extracted JS is executed

   after the chained JsExps have all been executed.

The last one is a fairly deep change, but should be manageable without regressions.

I don't like the other two much, personally, but wanted to open up the floor to other ideas.

One more tidibt: an example.

  JqId(Str("test")) ~>

    JqAppend(<button onclick="doSomething();">Test</button>) ~>

    JqAppend(<button onclick="doSomethingElse();">Test2</button>)

The goal here is:

  <div id="test">

    <button>Test</button>

    <button>Test2</button>

  </div>

Where the buttons do the appropriate thing. Option (a) would produce this JS:

  $('#test')

    .append((function() {

      lift.onEvent("lift-event-js-F397700660826TXLHFS", "click", function (event) {

        doSomething();

     });

     return "<button id=\"lift-event-js-F397700660826TXLHFS\">Test</button>";

    })())

    .append((function() {

      lift.onEvent("lift-event-js-F397700660826TXABCD", "click", function (event) {

        doSomething2();

     });

     return "<button id=\"lift-event-js-F397700660826TXABCD\">Test2</button>";

    })())

Note that `onEvent` runs before the HTML is returned, and therefore before the

append is executed. This is what happens today.

Option (b) would produce this JS:

  $('#test')

    .append("<button onclick=\"doSomething()\;">Test</button>")

    .append("<button onclick=\"doSomething2();\">Test2</button>");

Note that it would do this even if extractInlineJavaScript was enabled. So that flag

could not be used to deal with ContentSecurityPolicy issues if you did AJAX interactions

that used JsExps.

Lastly, option (c) would produce this JS:

  $('#test')

    .append("<button id=\"lift-event-js-F397700660826TXLHFS\">Test</button>")

    .append("<button id=\"lift-event-js-F397700660826TXABCD\">Test2</button>");

  lift.onEvent("lift-event-js-F397700660826TXLHFS", "click", function (event) {

    doSomething();

  });

  lift.onEvent("lift-event-js-F397700660826TXABCD", "click", function (event) {

    doSomething2();

  });

Note that here we run the event attachment after all the jQuery work is complete.

So, I vote (c). Other votes and/or ideas for handling the issue?

Thanks,

Antonio

[Matt Farmer]

Could we ship (b) for 3.1 and (c) in a later release? The current state of things sounds a bit scary, and doing this would give us plenty of time to test and vet things during the 3.2 cycle.

We could also ship (b) as a 3.0.2 if we feel the need is bad enough. Not sure where I fall on that at the moment but open to it.

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Antonio Salazar Cardozo]

I'm not too concerned about the current state because inline JS extraction is disabled by default and still has several edge cases, so it's essentially a WIP anyway. But certainly we could look at doing that as an intermediate step; it should be a fairly easy change (probably just passing a false to the HTML normalizer in fixHtmlFunc). 

Thanks,

Antonio 

You received this message because you are subscribed to a topic in the Google Groups "Lift" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/liftweb/S_vrVG9FIgk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to liftweb+u...@googlegroups.com.

[Matt Farmer]

Word. Well, I'm +1 on shipping that solution for 3.1 because WIP or not shipping things that are less broken is still ideal. And then doing the major overhaul for 3.2. =)

[Antonio Salazar Cardozo]

Updated https://github.com/lift/framework/issues/1801 with that decision and an initial

target of RC1.

Thanks,

--

Antonio Salazar Cardozo

On twitter @lightfiend

https://keybase.io/shadowfiend

[Antonio Salazar Cardozo]

Implemented the stopgap in https://github.com/lift/framework/pull/1842 .

Thanks,

Antonio

Thanks,

To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---

You received this message because you are subscribed to a topic in the Google Groups "Lift" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/liftweb/S_vrVG9FIgk/unsubscribe.

To unsubscribe from this group and all its topics, send an email to liftweb+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to the Google Groups "Lift" group.

To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code

---
You received this message because you are subscribed to a topic in the Google Groups "Lift" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/liftweb/S_vrVG9FIgk/unsubscribe.

To unsubscribe from this group and all its topics, send an email to liftweb+unsubscribe@googlegroups.com.