|
Description
Magnus Enger
2015-06-15 18:58:29 UTC
Jesse Weaver asked that this be assigned to him. Created attachment 40156 [details] [review] Bug 14389 - Editing a syspref in a textarea does not enable the Save button Test plan: 1. Navigate to the "opaccredits" syspref (or any other textarea, i.e., "Click to Edit", syspref) in the system preferences editor. 2. Change its contents, by either pasting or typing. The field may not be marked as modified, even after you click outside the box. 3. Apply the patch. 4. Reload the page and try again; either pasting or typing should mark the field as changed and allow you to save. Hi Jesse, I tested this, and the following action: highlighting everything, and deleting the contents of a syspref (this case, nologininstructions) the button didn't activate when I might have expected it to. I can get around it by adding and deleting a space in the syspref. Also, we're still stuck with the "middle click paste" from linux not triggering the save button, but that might be an even more distant edge case. Is this something we can fix? This patch is certainly an improvement over the current behaviour. Liz Hi Jesse, I just tried this with only 'input', and it seems to work in every way I'd expect it to, including the linux middle button paste - what do you think? Liz actually forget I said anything :) oy. it's getting too late for me, comment 4 stands, actually. Created attachment 40182 [details] [review] Bug 14389 - Editing a syspref in a textarea does not enable the Save button Test plan: 1. Navigate to the "opaccredits" syspref (or any other textarea, i.e., "Click to Edit", syspref) in the system preferences editor. 2. Change its contents, by either pasting or typing. The field may not be marked as modified, even after you click outside the box. 3. Apply the patch. 4. Reload the page and try again; either pasting or typing should mark the field as changed and allow you to save. Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> Note: "even after you click outside the box." => This works even without this patch. With this patch, the "save" button is enabled when a change is made on input and textarea without having to click outside of the element. Created attachment 40344 [details] [review] Bug 14389 - Editing a syspref in a textarea does not enable the Save button Test plan: 1. Navigate to the "opaccredits" syspref (or any other textarea, i.e., "Click to Edit", syspref) in the system preferences editor. 2. Change its contents, by either pasting or typing. The field may not be marked as modified, even after you click outside the box. 3. Apply the patch. 4. Reload the page and try again; either pasting or typing should mark the field as changed and allow you to save. Created attachment 40345 [details] [review] Bug 14389 - Editing a syspref in a textarea does not enable the Save button Test plan: 1. Navigate to the "opaccredits" syspref (or any other textarea, i.e., "Click to Edit", syspref) in the system preferences editor. 2. Change its contents, by either pasting or typing. The field may not be marked as modified, even after you click outside the box. 3. Apply the patch. 4. Reload the page and try again; either pasting or typing should mark the field as changed and allow you to save. Signed-off-by: Jesse Weaver <pianohacker@gmail.com> Confirmed working for normal input, paste and middle-click paste in Chrome and Firefox in Linux. Created attachment 40361 [details] [review] Bug 14389 - Editing a syspref in a textarea does not enable the Save button Test plan: 1. Navigate to the "opaccredits" syspref (or any other textarea, i.e., "Click to Edit", syspref) in the system preferences editor. 2. Change its contents, by either pasting or typing. The field may not be marked as modified, even after you click outside the box. 3. Apply the patch. 4. Reload the page and try again; either pasting or typing should mark the field as changed and allow you to save. Signed-off-by: Jesse Weaver <pianohacker@gmail.com> Confirmed working for normal input, paste and middle-click paste in Chrome and Firefox in Linux. Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> Hi Jonathan, should this be Passed QA? I was thinking about this bug recently. I suppose it still won't work for WYSIWYG editors in some cases, but that's not a community concern (yet). I'll have to do some thinking on that one when I have time... (In reply to David Cook from comment #12) > I was thinking about this bug recently. > > I suppose it still won't work for WYSIWYG editors in some cases, but that's > not a community concern (yet). I'll have to do some thinking on that one > when I have time... Oh wait. Liz pointed out that it's on "input" and not on "change". It might work for WYSIWYG editors as well! I'll look into this! (In reply to Katrin Fischer from comment #11) > Hi Jonathan, should this be Passed QA? No, I signed it off. (In reply to David Cook from comment #13) > (In reply to David Cook from comment #12) > > I was thinking about this bug recently. > > > > I suppose it still won't work for WYSIWYG editors in some cases, but that's > > not a community concern (yet). I'll have to do some thinking on that one > > when I have time... > > Oh wait. Liz pointed out that it's on "input" and not on "change". > > It might work for WYSIWYG editors as well! I'll look into this! (In reply to David Cook from comment #13) > (In reply to David Cook from comment #12) > > I was thinking about this bug recently. > > > > I suppose it still won't work for WYSIWYG editors in some cases, but that's > > not a community concern (yet). I'll have to do some thinking on that one > > when I have time... > > Oh wait. Liz pointed out that it's on "input" and not on "change". > > It might work for WYSIWYG editors as well! I'll look into this! So I've been doing lots of research (as I do). Locally, I've added TinyMCE to some system preferences which use HTML. (I plan to upstream it when I find some time.) I've just been using "keyup" to detect changes to the TinyMCE editor, and then I trigger a "keyup" on the original textarea so that the "Save..." button activates. It works OK for the most part, but I've noticed a few issues like changes from the editor buttons not being detected or pastes not made through Ctrl+V or Ctrl+Ins. Anyway... using "input" for an event listener has been absolutely great with Firefox and Chrome. It fires for so many different events. It's so awesome! Unfortunately, IE doesn't support the "input" event for elements that are "contenteditable". TinyMCE uses a "contenteditable" body element within an iframe element to create its editor, so IE and TinyMCE are just not going to be friends with the "input" event. But it looks like the "input" event works well with IE otherwise, so yay! Just adding this info here in case its somehow relevant to people in the future (plus it's also just painfully interesting to know). Sorry to spam people, but here are some relevant links for what I was talking about: https://developer.mozilla.org/en-US/docs/Web/Events/input https://connect.microsoft.com/IE/feedbackdetail/view/794285 Not actually relevant to this bug report though... just making that clear. This patch looks super good :) Created attachment 40772 [details] [review] Bug 14389 - Editing a syspref in a textarea does not enable the Save button Test plan: 1. Navigate to the "opaccredits" syspref (or any other textarea, i.e., "Click to Edit", syspref) in the system preferences editor. 2. Change its contents, by either pasting or typing. The field may not be marked as modified, even after you click outside the box. 3. Apply the patch. 4. Reload the page and try again; either pasting or typing should mark the field as changed and allow you to save. Signed-off-by: Jesse Weaver <pianohacker@gmail.com> Confirmed working for normal input, paste and middle-click paste in Chrome and Firefox in Linux. Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Patch pushed to master. Thanks Liz! Pushed to 3.20.x will be in 3.20.2 Pushed to 3.18.x will be in 3.18.09 Pushed to 3.16.x, will be in 3.16.13 *** Bug 10674 has been marked as a duplicate of this bug. *** |