Jump to content

Controllers can be initialized on detatched DOM nodes if another controller uses cleanContentsOf


Recommended Posts

If you have something like

<div data-controller="foo">
    <div data-controller="bar"></div>
</div>
ips.controller.register("foo", {
  initialize: function() {
    ips.controller.cleanContentsOf(this.scope);
  }
});

The controller on bar will be initialized on the dead element, and never cleaned up:

ips.controller.register("bar", {
  initialize: function() {
    console.log(document.contains(this.scope[0])); // => false
  },
  destroy: function() {
    console.log("destroyed"); // never called
  }
});

This happens because ips.controller#_findControllers is called once to get all controllers to load, then the controllers are run one by one. If the foo controller is run first (and I'm not sure if that's guaranteed to be the case), cleanContentsOf will drop the bar element, but it won't run the destructor because the controller hasn't been initialized yet. When all of that logic has finished running, it then runs the bar controller, even though it is no longer applicable.

Sure, you might say, but nobody would call cleanContentsOf synchronously during init, right? Yes, you do.

Concretely, the problem this causes me is that the commentFeed of a status update exists twice, one in the document and one detatched. This means addToCommentFeed events end up getting processed by both, the detached one first, and the detached one ends up throwing an exception.

There are a few different ways this could be fixed:

  • Fix the double initialization of profiles. Seems straight forward enough, and that appears to be the only instance of this issue at the moment.
  • Guard ips.controller#initControllerOnElem with node.contains(elem), to ensure that the node is still a child
  • Run the destructor code from cleanContentsOf in a `setImmediate` block, to give the other controllers an opportunity to be initialized before getting cleaned up
Link to comment
Share on other sites

  • 4 weeks later...
  • 4 weeks later...
  • 2 weeks later...
  • 1 month later...
  • 4 weeks later...
  • 1 month later...

@Marc Stridgen any news about this bug? It's open since July 2022. I thought it's going to be finally fixed by Jan 2023 release since it's only for 

Quote

During the holiday period, our focus is on providing bug fixes and improving stability. Feature updates will resume in February.

From version 4.5 this bug is exist. Now we are 4.7.7, and yet this bug report still open!!!

Link to comment
Share on other sites

  • 2 weeks later...

Just for the record I have found my comments where I reported this issue on July 25, 2021, and today is almost Feb 1st, 2023. I did before my comment in here on July 25, 2021, opened a support ticket where IPS stuff didn't even investigate this issue and blamed the third-party apps/plugins.

This issue has been reported for 1 years, 6 months, and 7 days as of today, and it is still not addressed even though @Colonel_mortis found what causing it. 

How long the problem will remain unfixed is unknown. @Matt

Link to comment
Share on other sites

When the editor freezes, I get:

front_front_statuses.js?v=4a97574f6d1675370690:4 
Uncaught TypeError: Cannot read properties of undefined (reading 'reset')
at baseController.addToStatusFeed (front_front_statuses.js?v=4a97574f6d1675370690:4:234)
at nr (root_library.js?v=4a97574f6d1675370690:1:7597)
at HTMLDocument.<anonymous> (root_library.js?v=4a97574f6d1675370690:1:8025)
at HTMLDocument.<anonymous> (root_library.js?v=4a97574f6d1675370690:1:1076)
at HTMLDocument.dispatch (root_library.js?v=4a97574f6d1675370690:2:43090)
at v.handle (root_library.js?v=4a97574f6d1675370690:2:41074)
at Object.trigger (root_library.js?v=4a97574f6d1675370690:2:71513)
at HTMLLIElement.<anonymous> (root_library.js?v=4a97574f6d1675370690:2:72108)
at Function.each (root_library.js?v=4a97574f6d1675370690:2:2976)
at jQuery.fn.init.each (root_library.js?v=4a97574f6d1675370690:2:1454)
Link to comment
Share on other sites

  • 3 months later...

Bumping this in the hope of bringing attention to this issue with 4.7.10 coming up. The most annoying way in which it manifests on my community is as in the quoted post:

On 7/5/2022 at 1:17 PM, Colonel_mortis said:

The symptom of this for regular status updates is that when you submit the status, it gets stuck on "Saving", despite having submitted successfully:

Could contain: Text, Word, Page, File

Our members and staff alike have been complaining about this for years and it hurts their perception of IPS and of our website. It's one more thing we have to explain to new users all the time, too ("yes, your status comment went through even though it looks frozen"), which hurts first impressions and creates unneeded anxiety about losing the comment they just spent the time writing.

Several approaches to fixing this have already been identified in this thread's OP. If you knock this out with your next release, it'll help all members of your customers' sites who use status updates.

Link to comment
Share on other sites

15 hours ago, SeNioR- said:

@Princess Celestia, this issue has been known for over a year or more and still not fixed. There is nothing to count on a patch because user statuses will be removed in IPS 5 anyway.

I know they'll be getting removed but v4 will still be supported until at least 2025 or 2026, which should mean bugfixes to existing functionality can still be expected until v4 support actually ends. And at this time, IPS still prominently promote it as a feature of their suite, which suggests they still intend to stand behind it at this time.

Status updates are an integral part of my community and I don't see us moving to v5 without them. I intend to license or build an add-on that restores them once v5 is out so I have an interest in the feature working well until then.

Link to comment
Share on other sites

  • 8 months later...
  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...