ILIAS-eLearning
UI, DataTable: 43621, show warning if URL too long
https://mantis.ilias.de/view.php?id=43621
The fix will build and check the URL when selecting rows; if the selection is done with the "+"-icon, a message will warn the user and stop selecting around the approximate limit. Checking rows manually will disable further checkboxes once this limit is met.
The main question, though, is: when is an URL actually to long? There are several issues involved, here:
browser limits
At first, I tried to determine the browser's limits by probing the url against the history (history.pushState({}, "", url);),
and found, that the browser is not our problem at all.
The common lists from the internet are well outdated, even Edge does not seem to care about URls > 8k.
server limits
However, ther server does care. NGinx and Apache block request about this size by default, so I increased the max chars in URLbuilder to 8192.
regex
That failed in building the URL, though. Depending on the nature of the url (number of params, encoded or not,...), the regex-steps to validate the query increase dramatically(!), up to an PREG_BACKTRACK_LIMIT_ERROR. My attempt of decreasing steps by modifying the regex had quite positive effects on some usecases, but broke others, and I stopped at some point. A way more simple approach is to split up parameters and check seperatly, although this will not amend a huge single param.
Hi @nhaagen,
not a request to actually try this, but just some hint that my curiosity brought up:
To prevent running into that PREG_BACKTRACK_LIMIT_ERROR (and even to speed up the matching...) we might use a technique like only once subpatterns. Maybe we want to look into this some other day...
Kind regards!
Hi @Amstutz, @thibsy,
please have a look here. This is not perfect, I guess, but better then what we currently have IMO.
Kind regards!
[ ] Ambiguous behaviour: the notice about long URLs is only displayed when selecting all rows using the corresponding action, but not when selecting rows individually. I think we should streamline this behaviour and display the notice for the latter scenario as well. Currently the checkboxes become disabled at some point without further notice, which could be confusing.
I had the warning for single-selection at first, but came annoyed by it almost immediately - the indication of disabled checkboxes should be quite easy to learn, but then I was testing around and both encountered the behavior frequently as well as I was expecting it. I do not have a too strong opinion about it, though, other than not to bother the assumed experienced user with too many alerts that she has to click away
Hi @nhaagen
@thibsy has asked me to look over the proposed solution. Thanks a lot for the new example in Data Table, this helps a lot to test the effects without having to create extra data sets. I think we should go with your implementation and do not show too many error messages.
I only have a rewording suggestion for the error message.
Please implement the following changes:
- [x] We should try to avoid technical explanations, because it can be assumed that users will not be able to understand/classify them. This is what I am currently reading from the error message, is that correct? „The number of selected lines results in too much data, which cannot be transmitted. Select less rows or perform the action for entire table (Bulk Actions).“
kindly, @yvseiler
@yvseiler / @Amstutz / @thibsy If you don't disagree (here or via some private channel) I would tend to follow @nhaagen|s experience for now and don't show explicit warnings when individual rows are selected.
We can indeed change this later if new experiences, ours or someone elses, suggest otherwise.
Sry, there's actually something wrong with the php 8.3 unit tests, please have a look.
@klees not entirely, sry - I was too hasty. The php 8.3 unit tests seem to fail because of the changes made to ILIAS\Data. I probably overlooked this because I am so used to having the php 8.4 unit tests fail now...
I just ran the tests again - don't know if some hickup or actual change, but they are passing now.