Wicket
  1. Wicket
  2. WICKET-5178

StopPropagation functionality on link is broken

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.7.0
    • Fix Version/s: 6.8.0, 7.0.0-M1
    • Component/s: None
    • Labels:
      None

      Description

      In the quickstart I'll add the following is illustrated:

      • simple table with AjaxLink on tr
      • on that tr there's another AjaxLink with should not propagate the onclick to the tr
      • So when clicking the "here" link, on the server logging only the following should appear:
      • onclick LNK
        but also the logging from the tr link is printed:
      • onclick LNK
      • onclick TR

      In wicket 6.6 this works.

      Thanks in advance ! Kind regards, Marieke

      1. myproject.zip
        21 kB
        Marieke Vandamme

        Issue Links

          Activity

          Hide
          Martin Grigorov added a comment -

          The change for 6.7 was intentional - WICKET-5093.
          AjaxRequestAttributes has a setting "allowDefault" which managed both JS event's "preventDefault" and "stopPropagation". It was a bug that it handled "stopPropagation".

          Maybe we should add a separate setting for stop propagation in AjaxRequestAttributes.

          Show
          Martin Grigorov added a comment - The change for 6.7 was intentional - WICKET-5093 . AjaxRequestAttributes has a setting "allowDefault" which managed both JS event's "preventDefault" and "stopPropagation". It was a bug that it handled "stopPropagation". Maybe we should add a separate setting for stop propagation in AjaxRequestAttributes.
          Hide
          Sven Meier added a comment -

          An additional setting for "stop propagation" would be nice, Java devs don't want to write "Wicket.Event.stop(attrs.event)".
          We could make that setting true by default, to make this change compatible with the previous behavior.

          Show
          Sven Meier added a comment - An additional setting for "stop propagation" would be nice, Java devs don't want to write "Wicket.Event.stop(attrs.event)". We could make that setting true by default, to make this change compatible with the previous behavior.
          Hide
          Martin Grigorov added a comment -

          Agree.

          Show
          Martin Grigorov added a comment - Agree.
          Hide
          Sven Haster added a comment -

          Seconded. Please fix asap. This broke functionality in an important page for us and we would not like to have to revert to wicket 6.6 for production.

          Show
          Sven Haster added a comment - Seconded. Please fix asap. This broke functionality in an important page for us and we would not like to have to revert to wicket 6.6 for production.
          Hide
          Martin Grigorov added a comment -

          It will be improved in 6.8.0.
          There is no need to revert to 6.6.0 for just this. There is explanation in WICKET-5093 how this can be handled.

          Show
          Martin Grigorov added a comment - It will be improved in 6.8.0. There is no need to revert to 6.6.0 for just this. There is explanation in WICKET-5093 how this can be handled.
          Hide
          Martin Grigorov added a comment -

          The new setting for AjaxRequestAttributes is added in master branch.
          Please have a look and let me know whether it fixes your problems.
          I'll test soon the attached quickstart with 6.x branch and backport the fix.

          Show
          Martin Grigorov added a comment - The new setting for AjaxRequestAttributes is added in master branch. Please have a look and let me know whether it fixes your problems. I'll test soon the attached quickstart with 6.x branch and backport the fix.
          Hide
          Sven Meier added a comment -

          Hi Martin,

          now allowDefault and stopPropagation are coupled once again, since you return false from the function:

          var handleStopPropagation = function (attributes) {
          var result = false;
          var evt = attributes.event;
          if (attributes.sp === "stop")

          { Wicket.Event.stop(evt); }

          else if (attributes.sp === "stopImmediate")

          { Wicket.Event.stop(evt, true); }

          else

          { result = true; }

          return result;
          };

          Is that you intention?

          Show
          Sven Meier added a comment - Hi Martin, now allowDefault and stopPropagation are coupled once again, since you return false from the function: var handleStopPropagation = function (attributes) { var result = false; var evt = attributes.event; if (attributes.sp === "stop") { Wicket.Event.stop(evt); } else if (attributes.sp === "stopImmediate") { Wicket.Event.stop(evt, true); } else { result = true; } return result; }; Is that you intention?
          Hide
          Martin Grigorov added a comment -

          I just improved it.
          Now they are separated.

          Show
          Martin Grigorov added a comment - I just improved it. Now they are separated.
          Hide
          Sven Meier added a comment -

          Separated but still not a fix for WICKET-5093:

          A delayed ajax call will call _handleEventCancelation() too late to have any effect on the browser's event processing .

          Show
          Sven Meier added a comment - Separated but still not a fix for WICKET-5093 : A delayed ajax call will call _handleEventCancelation() too late to have any effect on the browser's event processing .
          Hide
          Martin Grigorov added a comment -

          OK, I'll move back the W.E.stop() call in Wicket.Ajax.ajax(), without the "return" which messes with #preventDefault().
          I don't have time now to properly test your quickstart but I think it is possible to solve the problem with the new setting.

          Show
          Martin Grigorov added a comment - OK, I'll move back the W.E.stop() call in Wicket.Ajax.ajax(), without the "return" which messes with #preventDefault(). I don't have time now to properly test your quickstart but I think it is possible to solve the problem with the new setting.
          Hide
          Sven Meier added a comment -

          Both preventDefault and stopPropagation have to be handled in Wicket.Ajax.ajax() before the call is possibly queued by the channel. Why not call _handleEventCancelation() from there?

          Show
          Sven Meier added a comment - Both preventDefault and stopPropagation have to be handled in Wicket.Ajax.ajax() before the call is possibly queued by the channel. Why not call _handleEventCancelation() from there?
          Hide
          Igor Vaynberg added a comment -

          on a side note please rename StopPropagation to Propagation

          {BUBBLE, STOP, STOP_IMMEDIATELY}

          . reading StopPropagation.NO makes my head assplode

          Show
          Igor Vaynberg added a comment - on a side note please rename StopPropagation to Propagation {BUBBLE, STOP, STOP_IMMEDIATELY} . reading StopPropagation.NO makes my head assplode
          Hide
          Sven Meier added a comment -

          The fix works fine for me, thanks!

          A minor nitpick:
          Even if an event is throttled, _handleEventCancelation() should be called nevertheless. Otherwise an event might trigger the default or bubble up because the throttled execution is delayed.

          — a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
          +++ b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
          @@ -1797,13 +1797,12 @@
          throttler.throttle(throttlingSettings.id, throttlingSettings.d,
          Wicket.bind(function ()

          { call.ajax(attributes); - Wicket.Ajax._handleEventCancelation(attributes); }

          , this));
          }
          else

          { call.ajax(attributes); - Wicket.Ajax._handleEventCancelation(attributes); }

          + Wicket.Ajax._handleEventCancelation(attributes);
          });
          });
          },

          Show
          Sven Meier added a comment - The fix works fine for me, thanks! A minor nitpick: Even if an event is throttled, _handleEventCancelation() should be called nevertheless. Otherwise an event might trigger the default or bubble up because the throttled execution is delayed. — a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js +++ b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js @@ -1797,13 +1797,12 @@ throttler.throttle(throttlingSettings.id, throttlingSettings.d, Wicket.bind(function () { call.ajax(attributes); - Wicket.Ajax._handleEventCancelation(attributes); } , this)); } else { call.ajax(attributes); - Wicket.Ajax._handleEventCancelation(attributes); } + Wicket.Ajax._handleEventCancelation(attributes); }); }); },

            People

            • Assignee:
              Martin Grigorov
              Reporter:
              Marieke Vandamme
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development