Fix DOM manipulation in covers.js

[xmorave2]

There is an issue with DOM manipulation: the current code does move image node with ajax loaded cover out of the link which makes the link not clickable.

[demiankatz]

Thanks for this, @xmorave2, and sorry for the slow response -- it's taken me a while to find time to test this!

Unfortunately, I'm not exactly sure how to set up a scenario that breaks on the dev branch but works here. So far, everything I have tried looks the same in both places.

We probably need a Mink test to cover this scenario -- if you can help me understand how to reproduce the problem, I may be able to contribute a test to this PR.

Additionally, there seems to be a separate problem where backlinks do not display correctly in search results (though they do appear to work on the record page). Is this known/expected? Should we also fix that while working on other problems here?

[xmorave2]

@demiankatz the bug scenario for us is:

1. enable ajax covers
2. go to search results
   -> without fix, the cover is not clickable
   -> with fix, the cover is clickable

for us, the backlinks on detail does work too, but we do not use backlinks on search results so I am does not face this problem, I'll try to reproduce it and see what to do with it

[demiankatz]

@xmorave2, in my setup, the cover was never clickable, with or without the fix. I just started up a test instance with default settings and used Demo:true as my cover setting in config.ini. Sorry if I'm missing/forgetting something important!

[demiankatz]

I should clarify: covers were never clickable when ajaxcovers was set to true. They worked when it was set to false.

[crhallberg]

I think that the complexity came from making sure that if the cover is inside the record link and a backlink_url is returned from AJAX, we don't end up with a link inside of a link. In that case, I think we want the image to link to the record but the text below it backlink_text is linked elsewhere.

I'll admit that cover.phtml could probably be updated to make this easier to update with JS and easier for all of this to be understood. @xmorave2, would you be interested in working on improving this or shall I take a shot?

[demiankatz]

@xmorave2, I spent some more time digging into this today, and I still can't figure out a way for this code to make any difference to behavior. Cover behavior was significantly revised in #3546 in an effort to improve accessibility, and at this point, it seems that if you turn on ajax covers, <a> tags are never generated, and covers are never linked. I'm not sure if this is a bug or a feature -- I confess I've kind of lost the plot at this point. In any case, I'm not sure how to move this forward at this point, and I'm not sure if there's a larger problem that needs to be solved. It could be that the whole cover system is in need of simplification and re-thinking, but a week before the release is not the time for that kind of effort. :-) Let me know if there's something obvious that I'm missing; otherwise, I think we need to push this to a future milestone so we have more time to figure out what needs to be done.

[crhallberg]

I did put some work into making this code cleaner and more readable, but ultimately, @demiankatz is correct. In the cover HTML and where it is used, the possibility for the image being in a link has been removed.

If any work needs to be done it is in the removal of the orphaned code. Something that can wait for v11.1.

[demiankatz]

Thanks for confirming, @crhallberg -- I'm moving the milestone forward so we can figure this out in more detail after the 11.0 release.