- Development - lists.drupal.org

[drupal-devel] [feature] Display comments on own tab like wikipedia (patch attached)
by chx 13 Sep '05

13 Sep '05

Issue status update for http://drupal.org/node/24804 Post a follow up: http://drupal.org/project/comments/add/24804 Project: Drupal Version: cvs Component: comment.module Category: feature requests Priority: normal Assigned to: chx Reported by: moshe weitzman Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/discuss_2.patch (6.71 KB) Comment URL is introduced. Instead of node/$nid#comment-cid we have comment/$nid#comment-cid Other minor fixes. chx Previous comments: ------------------------------------------------------------------------ Sat, 11 Jun 2005 20:57:10 +0000 : moshe weitzman Attached please find a demonstration patch which moves comment display to its own tab on the node view page. Admin may choose this new view or old view. The patch needs work still: - comment control form redirects to the node page after submit. Should be comment page - same for comment posting form - probably more I hope someone gets inspired to finish this patch. I won't revisit this for a while. ------------------------------------------------------------------------ Sat, 11 Jun 2005 21:00:56 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/discuss.patch (3.87 KB) with attachment this time ------------------------------------------------------------------------ Sat, 11 Jun 2005 23:24:12 +0000 : moshe weitzman oh yeah - also on the TODO list is to comment URLs to always point to comment.module and never to node/x#comment-cid or node/x#comment-new and so on. then comment.module can work out whether to show you the discuss tab or the traditional view. ------------------------------------------------------------------------ Sun, 12 Jun 2005 00:56:02 +0000 : Boris Mann I like the concept. Would want this to be part of workflow settings, so that comment-tab could be enabled/disabled per content type. ------------------------------------------------------------------------ Sun, 12 Jun 2005 02:20:42 +0000 : Jaza I like the idea - big +1. This is a must for sites that have long articles and/or many comments - viewing the node is enough bandwidth as it is, without all the comments getting displayed as well. Also improves usability, IMO. Would also like to see pagination of the comments tab (I'm sure you've thought of this already, Moshe). Something similar to A List Apart [1]'s system, e.g. node/x/discuss/2 (page 2 of that node's comments). Or, if the existing pager system demands it, node/x/discuss?page=2. [1] http://www.alistapart.com/ ------------------------------------------------------------------------ Tue, 30 Aug 2005 00:41:42 +0000 : chx Attachment: http://drupal.org/files/issues/discuss_0.patch (3.87 KB) Let's discuss! Like do we want to have a reply form on the node display even if the comments are on the discuss page? ------------------------------------------------------------------------ Tue, 30 Aug 2005 00:50:13 +0000 : chx Attachment: http://drupal.org/files/issues/discuss_1.patch (3.58 KB) I have uploaded moshe's patch. Let's try again. ------------------------------------------------------------------------ Tue, 30 Aug 2005 11:22:18 +0000 : Bèr Kessels I like this approach a lot. However, as i use drupal also for blogging, would dislike my bog to behave the same. Thus there must be some -easy- way to get around this. µ The problem lies mainly in the fact that tabs are not administratable in Drupals menu admin. And I think it is a very bad idea to add a config option where to place the comments. We should go for one way, teh default way, and offer a menu-admin of some sort to change this, for bloggers and slashdot alikes. How to achieve this? I see two options, but I really hope people see more then one.: - make local-tasks appear in the menu admin (gets a huge +1 from me) It's a separate issue, though. - add a config option to the comment admin to allow comments either as tab or as thread under the post. IMO that config option should not set a variable, that gets checked every hook_menu call, but it should alter the menu-entry in the database for the tabs Its possible, I did that before. But, as said, I hope others have better ideas. for I really would love this feature to get in. But I am afraid a huge lot of people will move away from drupal if we allow tabs only. ------------------------------------------------------------------------ Tue, 30 Aug 2005 14:05:59 +0000 : chx No, not tabs only. I know the drupal_gotos are giving you this impression, but will fix in the evening. Also, there is only a variable_get for node/nid pages and then again, variable_get is double cached, it's very fast. ------------------------------------------------------------------------ Tue, 30 Aug 2005 16:19:22 +0000 : Bèr Kessels I know variable is fast, but I was referring to: && variable_get('comment_task', FALSE) that is evaluated every node view. As well as the setting in the comment options. I am no fan of such options; However, it seems in this case that setting is sortof required, unfortunately. So unless others come up with better ideas, just ignore my previous comment/post. Another idea: put this in hook_nodeapi() $op == "settings". that way this setting can be used per type. I can imagine that this makes no sense for forums and blog entries, yet is very usefull for the books on that same site. ------------------------------------------------------------------------ Tue, 30 Aug 2005 16:20:36 +0000 : Bèr Kessels sorry, with "Another idea: put this in..." I meant to say "Another idea: put the settings for tabs vs threads ..."

1 0

[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by m3avrck 13 Sep '05

13 Sep '05

Issue status update for http://drupal.org/node/30220 Post a follow up: http://drupal.org/project/comments/add/30220 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck Status: patch (code needs review) Attachment: http://drupal.org/files/issues/drupal_15.patch (1.94 KB) After talking with Drumm on IRC, this patch makes the integration with Drupal much simpler and it's off by default. Only turned on for the node edit/change page right now but any module/form can easily add this by passing a 'TRUE' parameter. m3avrck Previous comments: ------------------------------------------------------------------------ Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes? Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript. Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well. If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :) ------------------------------------------------------------------------ Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this. Cheers ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well? Michelle ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles Great for us multitaskers that rush and click the wrong thing ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:36:52 +0000 : chx Very strong -1 as this is IE only. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium Konqueror already does this, +1 for the idea in general. ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:07:21 +0000 : webchick Yeah, I think chx is correc that onbeforeunload is an extension developed by MS [1] (it's not in the ECMAScript specification [2]. However, I found out that this is implemented in Mozilla since 1.7 [3] (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in: Firefox 1.0.6 (working) Firefox Deer Park Alpha 1 (working) Opera 8.0.2 (working) Safari 1.3 (does NOT work) Safari 2.0.1 (working) So it seems to be a 'de facto' standard if nothing else. [1] http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun… [2] http://www.ecma-international.org/publications/standards/Ecma-262.htm [3] http://www.mozilla.org/status/2004-03-01.html ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:09:15 +0000 : webchick Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes) And a patch is ready! Code was based on many example, including this [4] and this [5] along with adding my own thoughts and what not ;) [4] http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O… [5] http://www.blogger.com/app/scripts/formcheck.js ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB) And here is the JS to throw in misc/. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB) Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know! ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB) One more try with those tabs. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman hmmm. i usually find these prompts annoying. any chance we can attach the behavior only to the node and comment forms? those are the "high risk" areas. what do others think? ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:42:39 +0000 : m3avrck Yes, the JS is only loaded on the node edit/create pages, won't affect any admin/login/etc form. Also, the prompt is unobstrusive and if you goto save a node you don't get a warning that the contents have changed because you are explicity saving the node (hence no reason!). ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:53:01 +0000 : Dries I want some JS-folks to review the code.

1 0

[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by Dries 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30220 Post a follow up: http://drupal.org/project/comments/add/30220 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: Dries Status: patch (code needs review) I want some JS-folks to review the code. Dries Previous comments: ------------------------------------------------------------------------ Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes? Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript. Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well. If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :) ------------------------------------------------------------------------ Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this. Cheers ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well? Michelle ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles Great for us multitaskers that rush and click the wrong thing ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:36:52 +0000 : chx Very strong -1 as this is IE only. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium Konqueror already does this, +1 for the idea in general. ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:07:21 +0000 : webchick Yeah, I think chx is correc that onbeforeunload is an extension developed by MS [1] (it's not in the ECMAScript specification [2]. However, I found out that this is implemented in Mozilla since 1.7 [3] (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in: Firefox 1.0.6 (working) Firefox Deer Park Alpha 1 (working) Opera 8.0.2 (working) Safari 1.3 (does NOT work) Safari 2.0.1 (working) So it seems to be a 'de facto' standard if nothing else. [1] http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun… [2] http://www.ecma-international.org/publications/standards/Ecma-262.htm [3] http://www.mozilla.org/status/2004-03-01.html ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:09:15 +0000 : webchick Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes) And a patch is ready! Code was based on many example, including this [4] and this [5] along with adding my own thoughts and what not ;) [4] http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O… [5] http://www.blogger.com/app/scripts/formcheck.js ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB) And here is the JS to throw in misc/. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB) Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know! ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB) One more try with those tabs. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman hmmm. i usually find these prompts annoying. any chance we can attach the behavior only to the node and comment forms? those are the "high risk" areas. what do others think? ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:42:39 +0000 : m3avrck Yes, the JS is only loaded on the node edit/create pages, won't affect any admin/login/etc form. Also, the prompt is unobstrusive and if you goto save a node you don't get a warning that the contents have changed because you are explicity saving the node (hence no reason!).

1 0

[drupal-devel] [bug] theme_image() doesn't actually output width and height of image
by Robrecht Jacques 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30935 Post a follow up: http://drupal.org/project/comments/add/30935 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: Robrecht Jacques Status: patch (ready to be committed) This is correct, the theme('image') for the screenshots need to have TRUE as last parameter, or omit the parameter (like the patch does). This last patch is ok. Didn't test it, but I'm sure it is correct. Without the width and height is will work too (it has before), but this is cleaner HTML. Patch is ready to commit. Robrecht Jacques Previous comments: ------------------------------------------------------------------------ Sun, 11 Sep 2005 16:22:28 +0000 : m3avrck Function theme_image() doesn't actually return a width and height for an image like it claims to do. ------------------------------------------------------------------------ Sun, 11 Sep 2005 19:30:35 +0000 : m3avrck Attachment: http://drupal.org/files/issues/drupal_11.patch (2.74 KB) Ok patched attached, which fixes this issue. Also, included a patch for system.module which sets the screen shots to 'TRUE' so image dimensions will also be outputted there as well (which they should be!). ------------------------------------------------------------------------ Sun, 11 Sep 2005 19:33:41 +0000 : m3avrck Attachment: http://drupal.org/files/issues/drupal_12.patch (2.75 KB) Fixed a tab issue. ------------------------------------------------------------------------ Sun, 11 Sep 2005 19:35:46 +0000 : m3avrck Attachment: http://drupal.org/files/issues/drupal_13.patch (2.75 KB) Fixed a spacing issue. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:18:32 +0000 : Souvent22 Used the patch, and did a quick test. Worked well for me. +1. ------------------------------------------------------------------------ Mon, 12 Sep 2005 16:21:16 +0000 : Robrecht Jacques I don't see why this patch is needed, "theme_image" returns a img tag with the width and height set if $getsize = TRUE. Eg: $node->body = theme('image', file_create_path('druplicon.png'), 'no alt', 'no title', array(), TRUE) . theme('image', file_create_path('druplicon.png'), 'no alt', 'no title', array(), FALSE) will return: <img src="files/druplicon.png" alt="no alt" title="no title" width="88" height="100" /> <img src="files/druplicon.png" alt="no alt" title="no title" /> (if druplicon.png is copied to the files/ directory). I don't see what you are fixing... You are right about the use of theme('image') in system.module though. The "false" should be "true". ------------------------------------------------------------------------ Mon, 12 Sep 2005 16:40:56 +0000 : m3avrck Well the actual code in theme_image() returned this: return '<img src="'. check_url($path) .'" alt="'. check_plain($alt) .'" title="'. check_plain($title) .'" '. $image_attributes . $attributes .'/>'; There is *no* mention of the $width and $height variables that are assigned above, not used at all. I checked images in the themes directory and none had this information, unless I was missing something obvious, I see no way that is being generated... nothing in the above img src about it. ------------------------------------------------------------------------ Mon, 12 Sep 2005 18:41:25 +0000 : Dries Taken from http://php.net/getimagesize: "Index 3 is a text string with the correct height="yyy" width="xxx" string that can be used directly in an IMG tag.". $image_attributes contains this information. Marking this fixed. Please reopen if not. ------------------------------------------------------------------------ Mon, 12 Sep 2005 18:49:33 +0000 : m3avrck Attachment: http://drupal.org/files/issues/system.module_10.patch (1.53 KB) Dries, great catch! What prompted this originally was that the screenshots on the theme page didn't have dimensions, didn't realize that index [3] returned this. Anyways, this patch fixes the screen shots and adds widths/heights.

1 0

[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by m3avrck 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30220 Post a follow up: http://drupal.org/project/comments/add/30220 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck Status: patch (code needs review) Yes, the JS is only loaded on the node edit/create pages, won't affect any admin/login/etc form. Also, the prompt is unobstrusive and if you goto save a node you don't get a warning that the contents have changed because you are explicity saving the node (hence no reason!). m3avrck Previous comments: ------------------------------------------------------------------------ Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes? Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript. Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well. If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :) ------------------------------------------------------------------------ Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this. Cheers ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well? Michelle ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles Great for us multitaskers that rush and click the wrong thing ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:36:52 +0000 : chx Very strong -1 as this is IE only. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium Konqueror already does this, +1 for the idea in general. ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:07:21 +0000 : webchick Yeah, I think chx is correc that onbeforeunload is an extension developed by MS [1] (it's not in the ECMAScript specification [2]. However, I found out that this is implemented in Mozilla since 1.7 [3] (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in: Firefox 1.0.6 (working) Firefox Deer Park Alpha 1 (working) Opera 8.0.2 (working) Safari 1.3 (does NOT work) Safari 2.0.1 (working) So it seems to be a 'de facto' standard if nothing else. [1] http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun… [2] http://www.ecma-international.org/publications/standards/Ecma-262.htm [3] http://www.mozilla.org/status/2004-03-01.html ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:09:15 +0000 : webchick Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes) And a patch is ready! Code was based on many example, including this [4] and this [5] along with adding my own thoughts and what not ;) [4] http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O… [5] http://www.blogger.com/app/scripts/formcheck.js ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB) And here is the JS to throw in misc/. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB) Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know! ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB) One more try with those tabs. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman hmmm. i usually find these prompts annoying. any chance we can attach the behavior only to the node and comment forms? those are the "high risk" areas. what do others think?

1 0

[drupal-devel] [task] Usability: Upload module needs a 'default' maxfile size setting
by m3avrck 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30025 Post a follow up: http://drupal.org/project/comments/add/30025 Project: Drupal Version: cvs Component: upload.module Category: tasks Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck -Status: patch (code needs review) +Status: patch (ready to be committed) Bump for this simple usability fix! m3avrck Previous comments: ------------------------------------------------------------------------ Tue, 30 Aug 2005 15:15:02 +0000 : m3avrck Problem: I've enabled upload.module I then later assign a few roles to be able to upload files. All of a sudden this new, hidden setting appears in Settings > Upload .. max file size and total file size for *each* role that has this capability. Hmm, good thing I decided to check back there! But where did those defaults come from, hmm. Fix: There should be a 'default' setting in Settings > Upload for max file size and total file size for *all* roles. Then, when I create a role and assign it upload permissions, it should inherit these defaults, which shouldn't be hardcode into Drupal anyways (as they are now). However, after I assign this permission, override settings should appear, as they currently do. Benefits: If I assign uploads to 20 different roles I can have default setting for them all instead of having to worry about changing them all. Also, this fixes the obviously usability problem and makes this module make more sense which is always a +1 :) ------------------------------------------------------------------------ Tue, 30 Aug 2005 15:20:59 +0000 : m3avrck Correction, there should also be a default setting for extensions as well. ------------------------------------------------------------------------ Tue, 30 Aug 2005 18:49:10 +0000 : m3avrck Attachment: http://drupal.org/files/issues/upload.module_3.patch (3.29 KB) Ok I've created a patch which fixes this problem. Thorougly test here, please test and let me know! This certainly does simplify the out-of-box experience with the upload.module. ------------------------------------------------------------------------ Tue, 30 Aug 2005 19:11:20 +0000 : m3avrck Attachment: http://drupal.org/files/issues/upload.module_4.patch (3.54 KB) Fixed a small bug caught by mozillman in IRC. ------------------------------------------------------------------------ Tue, 30 Aug 2005 20:01:49 +0000 : Souvent22 Tried patch. Seems to work ok. +1 ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:30:06 +0000 : walkah +1 - perhaps i'm getting old and foggy - but isn't this how upload.module used to work?? ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:32:34 +0000 : Junyor There used to be a setting for max file size for the whole site, but I don't think these defaults were listed anywhere. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:34:37 +0000 : m3avrck I think originally there was a setting similiar to this however I'm not sure if it was setting a 'Default' value or a 'Global' value for all uploads. I think that is what the originaly problem was for taking it out. This patch, coupled with my other patch: http://drupal.org/node/30038 (rerolled if/when Form API gets in) makes the administration of the upload settings page *very* easy and super clean and intuitive as well.

1 0

[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by moshe weitzman 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30220 Post a follow up: http://drupal.org/project/comments/add/30220 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: moshe weitzman -Status: patch (code needs work) +Status: patch (code needs review) hmmm. i usually find these prompts annoying. any chance we can attach the behavior only to the node and comment forms? those are the "high risk" areas. what do others think? moshe weitzman Previous comments: ------------------------------------------------------------------------ Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes? Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript. Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well. If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :) ------------------------------------------------------------------------ Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this. Cheers ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well? Michelle ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles Great for us multitaskers that rush and click the wrong thing ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:36:52 +0000 : chx Very strong -1 as this is IE only. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium Konqueror already does this, +1 for the idea in general. ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:07:21 +0000 : webchick Yeah, I think chx is correc that onbeforeunload is an extension developed by MS [1] (it's not in the ECMAScript specification [2]. However, I found out that this is implemented in Mozilla since 1.7 [3] (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in: Firefox 1.0.6 (working) Firefox Deer Park Alpha 1 (working) Opera 8.0.2 (working) Safari 1.3 (does NOT work) Safari 2.0.1 (working) So it seems to be a 'de facto' standard if nothing else. [1] http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun… [2] http://www.ecma-international.org/publications/standards/Ecma-262.htm [3] http://www.mozilla.org/status/2004-03-01.html ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:09:15 +0000 : webchick Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes) And a patch is ready! Code was based on many example, including this [4] and this [5] along with adding my own thoughts and what not ;) [4] http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O… [5] http://www.blogger.com/app/scripts/formcheck.js ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB) And here is the JS to throw in misc/. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB) Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know! ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB) One more try with those tabs.

1 0

[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by m3avrck 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30220 Post a follow up: http://drupal.org/project/comments/add/30220 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck Status: patch (code needs work) Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB) One more try with those tabs. m3avrck Previous comments: ------------------------------------------------------------------------ Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes? Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript. Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well. If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :) ------------------------------------------------------------------------ Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this. Cheers ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well? Michelle ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles Great for us multitaskers that rush and click the wrong thing ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:36:52 +0000 : chx Very strong -1 as this is IE only. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium Konqueror already does this, +1 for the idea in general. ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:07:21 +0000 : webchick Yeah, I think chx is correc that onbeforeunload is an extension developed by MS [1] (it's not in the ECMAScript specification [2]. However, I found out that this is implemented in Mozilla since 1.7 [3] (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in: Firefox 1.0.6 (working) Firefox Deer Park Alpha 1 (working) Opera 8.0.2 (working) Safari 1.3 (does NOT work) Safari 2.0.1 (working) So it seems to be a 'de facto' standard if nothing else. [1] http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun… [2] http://www.ecma-international.org/publications/standards/Ecma-262.htm [3] http://www.mozilla.org/status/2004-03-01.html ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:09:15 +0000 : webchick Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes) And a patch is ready! Code was based on many example, including this [4] and this [5] along with adding my own thoughts and what not ;) [4] http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O… [5] http://www.blogger.com/app/scripts/formcheck.js ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB) And here is the JS to throw in misc/. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB) Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know!

1 0

[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by m3avrck 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/30220 Post a follow up: http://drupal.org/project/comments/add/30220 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck Status: patch (code needs work) Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB) Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know! m3avrck Previous comments: ------------------------------------------------------------------------ Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes? Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript. Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well. If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :) ------------------------------------------------------------------------ Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this. Cheers ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well? Michelle ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles Great for us multitaskers that rush and click the wrong thing ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:36:52 +0000 : chx Very strong -1 as this is IE only. ------------------------------------------------------------------------ Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium Konqueror already does this, +1 for the idea in general. ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:07:21 +0000 : webchick Yeah, I think chx is correc that onbeforeunload is an extension developed by MS [1] (it's not in the ECMAScript specification [2]. However, I found out that this is implemented in Mozilla since 1.7 [3] (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in: Firefox 1.0.6 (working) Firefox Deer Park Alpha 1 (working) Opera 8.0.2 (working) Safari 1.3 (does NOT work) Safari 2.0.1 (working) So it seems to be a 'de facto' standard if nothing else. [1] http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun… [2] http://www.ecma-international.org/publications/standards/Ecma-262.htm [3] http://www.mozilla.org/status/2004-03-01.html ------------------------------------------------------------------------ Mon, 12 Sep 2005 15:09:15 +0000 : webchick Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers. ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes) And a patch is ready! Code was based on many example, including this [4] and this [5] along with adding my own thoughts and what not ;) [4] http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O… [5] http://www.blogger.com/app/scripts/formcheck.js ------------------------------------------------------------------------ Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB) And here is the JS to throw in misc/.

1 0

[drupal-devel] [bug] Comments per page, links don't go to correct page
by killes 12 Sep '05

12 Sep '05

Issue status update for http://drupal.org/node/26966 Post a follow up: http://drupal.org/project/comments/add/26966 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: jakeg Updated by: killes(a)www.drop.org -Status: active +Status: patch (code needs review) The first patch is an obvious bugfix and shoudl be committed. I don't understand the rest of the probem, though. paged coments have been broken for a long time, so it would be nice to see them fixed. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Mon, 18 Jul 2005 15:53:36 +0000 : jakeg When you, as admin, set the default number of comments per page, the links to those comments are not updated as they should be. For example, all links to individual comments still go to /node/x/#comment-x but comment no. x may not be on that page if x is bigger than the number of comments per page. Hence the link is wrong. This includes links from the 'recent comments' block and also very importantly after someone has submitted a new comment, they should be taken to their comment but if their new comment isn't on the first page then they have the same problem. This is a *big* problem if you have discussion/comment threads of more than about 100 posts (some threads on my site are over 300 posts) and hence you can't sensibly include them all on one page. ------------------------------------------------------------------------ Wed, 27 Jul 2005 20:44:29 +0000 : jakeg bump. I consider this to be quite important. Its definitely a 'bug' and not a feature request as when you click a link to a certain comment you expect to get to that comment, as the # in the URL would suggest. ------------------------------------------------------------------------ Tue, 09 Aug 2005 08:19:03 +0000 : jakeg bump - can anyone help? Its a major problem for me. ------------------------------------------------------------------------ Sun, 21 Aug 2005 06:58:32 +0000 : robertDouglass Changing this to CVS and widening the scope. When I set the comments per page at 10, in the CVS version, and then make a node with over 10 comments, I get no pager links to other pages of comments, but rather a useless "moderate comments" button. In short, in my install from yesterday, comments are foobar. ------------------------------------------------------------------------ Thu, 25 Aug 2005 23:49:03 +0000 : tostinni Attachment: http://drupal.org/files/issues/comment_page.patch (1.02 KB) "When I set the comments per page at 10, in the CVS version, and then make a node with over 10 comments, I get no pager links to other pages of comments, but rather a useless "moderate comments" button. In short, in my install from yesterday, comments are foobar. " Ok I got a patch for this, it's just a smal mistake in the order of the pager_query arguments. I'll have a look at the comment link tomorrow. ------------------------------------------------------------------------ Fri, 26 Aug 2005 22:49:52 +0000 : tostinni Regarding the block comment : "When you, as admin, set the default number of comments per page, the links to those comments are not updated as they should be. For example, all links to individual comments still go to /node/x/#comment-x but comment no. x may not be on that page if x is bigger than the number of comments per page. Hence the link is wrong. " In HEAD, it seems that links goes to /node/$nid#comment-$cid so I think there's a mistake here, it should be /node/$nid/$cid#comment-$cid if you want to see the individual comment. But if we want to see global comment thread and move the focus to the current comment, we should consider doing something like node/$nid?page=$pid&comments_per_page=10#comment-$cid $nid, $pid, $cid are variable defining the comment. What do you think about this ? ------------------------------------------------------------------------ Tue, 30 Aug 2005 17:29:51 +0000 : tostinni Attachment: http://drupal.org/files/issues/comment_page_0.patch (2.32 KB) Here comes a new patch, and for the moment, here will stop my contributions. I need help and review because I don't know how to fix the next bugs. What do my patch : - correct the pager_query function to see the list of comment as described in #4 - correct the links on the title of the comment when seeing the comments. Currently they are built like : /$nid#comment-$cid this doesn't work when a page is specified, so we have to look if URL get extra parameters like "page=1" to rebuilt the url link. So I modified phptemplate engine to change the comment theme function in order it takes these parameters. Now I'm still blocked because I don't have any idea how to solve the biggest problems in comment_block and about redirecting user after posting his comment. This is not so hard to tell, but I don't know a clean way to do this : In comment_block, the problem is that when we make the link to last comment, we didn't know the page where the post is supposed to be (this only happen when we have a threaded mode). So we should find it. Yes, but the problem is how... We should do an other query, like the one we made in comment_render and then find the position in thread of the current comment to add the page fragment to the url. It happens pretty much the same in the redirection after writing a comment. Any odeas on how we should make this ? 4.7 is coming... :)

1 0