> > 2) In the great religious debate between whether braces are > required > > for single-line blocks.they are. J Minor point. > > I was looking through the existing code and found it both ways in > different places... will fix.
Yeah, it's still pretty inconsistent throughout the project, although every file I substantially changed, I fixed up. > > 5) You're slurping the whole file into memory before writing it > out. > > While I'm no fan of premature optimization, this does seem like a > good > > vector for a DoS attack. Then again, it's no different than the > wikitext we > > let people post. > > Actually the version without Attachment also has the same behaviour. > The normal Edit form sends all of the original text and all of the > current text. I needed to mimic this behaviour. If I have time I would > replace this with an MD5 or SHA1 hash value, or use the HtmlControl > variables in the pagebehind. Yeah, I don't have a problem with that. It's not awesome, but as you say it's no worse than the existing stuff. > > 7) What's the purpose of putting different file types in > different > > directories? Is it just to organize things on the server, i.e. > convenience? > > That was the way the original upload code from the FlexWiki site did > it so I kept it. This is available for discussion. There is no real > reason in today's file systems. also if I was to make it possible to > store the uploads in SQL Server it would all go in the same table. So this raises a two interesting questions, one important and one not: * (Not important) Is it necessary to match what's there with the original upload code? I can't think of a good reason to, especially as the 1.8->2.0 transition is our chance to introduce breaking changes (within reason). However, others who are using this feature might have different opinions. * (Important) Are attachments a feature of the web app or a feature of the core? If they're a feature of the core, then we should probably support storing attachments in whatever store has been defined - filesystem, SQL Server, whatever else. That will require modification of the core components (security, caching, etc.). If it's a feature of the web app, then it may be incompatible or inconsistent with non-filesystem stores, but it'll be easier to code. > > 8) You really limit the system to nine versions of a file? Is > there a > > reason for that? > > Again that was what the original code did. I can increase to 99 > easily, it is more difficult to make it unlimited, but not impossible. I think this follows from the question of whether to integrate the functionality into the core or not. If it goes into the core, it should probably follow the same (or similar) versioning semantics that topics have. > > > > 9) On my machine, when I click "Show Attachment Controls", the > right > > border slides off the screen, making me scroll. That's a bit > inconvenient. > > That happens in Firefox, at least 2.0.0.4. It does not happen in IE > 7.0.5730.11. It is behaviour like this that drives web programmers > nuts. I need more time to work out why it behaves this way or find a > fix. I agree it is annoying (Note IE does not give any scrollbars and > some of the controls will end up below the window, making it necessary > to blindly navigate to these hidden controls by tabbing.) Well, from my perspective this is your call. That is, whether to release it as-is or not is up to you. Personally I find it pretty annoying, but I'd probably turn off attachments on my wikis anyway for other reasons. > If the flexwiki.config does not contain the setting for ContentUpload > (i don't have the exact name available), the Attachment controls > should not display or be accessible. I agree this was not my first > design choice, but to make so that the control is not output to the > page requires a full redesign of the EditWiki page. The use of the > construct <% DoPage(); %> in the aspx file it means that all controls > are placed on the page during the PageRender event. The HtmlInputFile > control _must_ be on the page at the PageLoad event, otherwise > security mechanisms kick in and prevent data being passed to the > pagebehind code. I almost started such a rewrite, but felt that would > have been presumptuous. Maybe there is an alternate solution that I am > not seeing? No, that's fine. We can just make it clear via the comment in the file that commenting out the item in flexwiki.config disables the feature. That's pretty reasonable. As far as the question of rewriting WikiEdit.aspx, I'd do you one better and suggest that the entire web app needs to be rewritten. It's a royal mess. It's not presumption on your part to do so, but I do think it would be smart of us to coordinate about timing. As we march (slowly but surely) towards 2.0, branching towards a 2.2 release is an option we should consider. > > > > 12) The "Send" and "Create Attachment" buttons are confusing to me > - it > > seems like "Create Attachment" inserts WikiTalk, but it's not clear - > both > > buttons imply "upload" to me. > > > The Send button causes the file to be uploaded to the server. The > Create Attachment emits the WikiTalk based on the input from the > controls exposed by the chosen radio button for format. Ah, I see. Maybe a way of visually separating those two things (another sidebar box?) would help make that clearer. Or maybe I'm just slow. :) > > > Because I was unable to get it working, I couldn't do much more > review than > > that. Do you have the feature working on a webserver somewhere in its > > current form? I'd love to hear what other people think of it, too. > > > A working version is available at (this is one without any > authentication controls): > > http://ods.dyndns.org/FlexWiki/default.aspx/OdsWiki/AttachmentPage.html Can you describe what you're doing for authentication? That's one of my concerns: we've added a new namespace-based security model to FlexWiki, and now we've given people the ability to upload content that's in no namespace. Reconciling those two things seems like it might be difficult, but maybe you've thought of something clever. Certainly it argues for integrating "attachment" capability into the core. I should also mention that I'm just asking questions, not saying how it "needs" to be done. I'd like to see some discussion on this feature, and I think we should strategize about what release it goes into, but I'm not going to hold it up just because it isn't done how I pictured it. In other words: comments from the community are needed - please let us know what you want to see here! ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Flexwiki-users mailing list Flexwiki-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flexwiki-users
