Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
Thanks for the comments! Please find my replies inline: On Tue, Apr 6, 2010 at 2:51 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 1, 2010, at 10:43 PM, Roland Steiner wrote: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). Which layout tests? Do they reference bugs? We can study the bugs to see if they truly ask for the behavior being tested. On Tue, Apr 6, 2010 at 3:39 PM, Justin Garcia justin.gar...@apple.com wrote: I think you're right except that if the table doesn't look like a table (like if it doesn't have any cell borders) its content can just look like any other set of paragraphs. Content should be merged in those cases I think. Off the top of my head I believe some Mail behavior depended on this. You could check and see when those layout tests were checked in and bug titles from the corresponding ChangeLog entries might shed some light on this. The following layout tests explicitly require outside content to be moved into table cells: editing/deleting/5032066.html rdar://problem/5032066 Delete should work between To Dos editing/deleting/delete-block-table.html (updated multiple times, the change marked with * is the same as for the above layout test) rdar://problem/4622763 Deleting from beginning of paragraph following a table deletes rather than selects the table Setup for rdar://problem/4344550 Misspellings aren't marked after undo delete rdar://problem/4922367 WebView selectLine: followed by deleteBackward: deletes TABLE element of following line * rdar://problem/5032066 Delete should work between To Dos rdar://problem/5107422 TOT REGRESSION: Delete key fails to delete text, and cursor disappears in Mail.app REGRESSION (r19595): WebViewDidBeginEditingNotification not posted when focusing with the mouse updated some test results that were affected by Hyatt's fix for rdar://problem/5208440 REGRESSION: Raw text needs to be pulled outside of tables (13753) editing/deleting/5206311-2.html rdar://problem/5206311 Whitespace can't be removed when editing text pasted into from web page A quick and dirty fix also affected the following layout tests: editing/deleting/5026848-2.html editing/deleting/5026848-3.html moving content into table cell doesn't seem to be the point of these tests, just a side-effect editing/deleting/5115601.html mentions table cell, but point of the test doesn't seem to be table-specific - moving content into table cell seems therefore a side effect editing/unsupported-content/table-delete-001.html current result image contradicts the expectation described in the test header, fix would create intended result editing/unsupported-content/table-delete-001.html current result seems to contain extraneous br, fix would remove it editing/inserting/12882.html unexpected image diff in editing region halo (not sure why) editing/execCommand/5432254-2.html ASSERTs (placeholder br seems to get deleted/pruned) Hard to comment on this idea from such a high level view. I don't understand how EditingPosition is meant to be different from VisiblePosition. Is EditingPosition just a VisiblePosition that's also a place where you can edit? I don't understand how DOMPosition is different in intent from the current Position. I'm not sure you want the low-level class to based on RangeBoundaryPoint, since the latter has the ability to adjust in response to DOM changes, which I am not totally sure we want in this case. The basic idea would mainly be to combine PositionIterator and Position into one class EditingPosition (or just Position), focusing on performance, and to move renderer-specific code into VisualPosition (which could be (re-)named RenderedPosition for clarity). Apart from an IMHO clearer code separation this should also help improving the handling of :before/:after content renderers, which currently is buggy. DOMPosition/RangeBoundaryPoint would continue to be just the bridge to JavaScript code and not used in (most) editing code. Non-contiguous selections suck as UI, except for special cases like selecting tables by column. I don't think they are a good way to highlight find results. I don't think you want to end up with a non-contiguous selection highlighting all results when you do a find. I don't understand: Safari and Chrome both do basically exactly that (albeit in
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Apr 7, 2010, at 12:11 AM, Roland Steiner wrote: Hard to comment on this idea from such a high level view. I don't understand how EditingPosition is meant to be different from VisiblePosition. Is EditingPosition just a VisiblePosition that's also a place where you can edit? I don't understand how DOMPosition is different in intent from the current Position. I'm not sure you want the low-level class to based on RangeBoundaryPoint, since the latter has the ability to adjust in response to DOM changes, which I am not totally sure we want in this case. The basic idea would mainly be to combine PositionIterator and Position into one class EditingPosition (or just Position), focusing on performance, and to move renderer-specific code into VisualPosition (which could be (re-)named RenderedPosition for clarity). Apart from an IMHO clearer code separation this should also help improving the handling of :before/:after content renderers, which currently is buggy. It's not clear to me how PositionIterator is the same concept as EditingPosition. The latter implies that it would only ever represent a position where you can edit. The former implies that it produces a sequence of positions (perhaps retaining additional state to be able to step forward/back efficiently). It also seems to me that it is useful to have a concept of a Position that is primarily optimized iteration. I'm also still not clear on the proposed relation between EditingPosition and VisiblePosition. Does every EditingPosition have an associated VisiblePosition? How about vice versa? Is the mapping one-to-one? DOMPosition/RangeBoundaryPoint would continue to be just the bridge to JavaScript code and not used in (most) editing code. Non-contiguous selections suck as UI, except for special cases like selecting tables by column. I don't think they are a good way to highlight find results. I don't think you want to end up with a non- contiguous selection highlighting all results when you do a find. I don't understand: Safari and Chrome both do basically exactly that (albeit in a somewhat souped-up fashion) - why is this capability good in the browser, but not for user scripts? Safari doesn't make this non-contiguous region a selection. It marks it with a visual highlight. Only the first hit is actually selected. This makes a big difference when doing a find in editable text - typing only overtypes the first hit, rather than replacing the non- contiguous selection. Being able to represent a non-contiguous region is interesting, but it would be UI hell to allow such a thing to actually act as the user selection, and to get copied and pasted. On Wed, Apr 7, 2010 at 3:53 AM, Justin Garcia justin.gar...@apple.com wrote: For example: divimg style=display:blockimg style=display:block/div [img1, 1] and [img2, 0] are different visually but would both be normalized to the same position under the above proposal. I think this is a great example and shows that normalizing [img, 0] to [parent-of-img, index-of-img] can probably only happen once styles and renderers are taken into account, which doesn't strictly contradict Darin's point though (?). I'm not sure how it implies that. Would you assign a real DOM position differently between the two styles? I think the real difference here is in affinity, i.e. whether the caret would be at the end of one line or the start of the next. However this doesn't affect the DOM parent and offset. Regardsm Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Wed, Apr 7, 2010 at 4:36 PM, Maciej Stachowiak m...@apple.com wrote: It's not clear to me how PositionIterator is the same concept as EditingPosition. The latter implies that it would only ever represent a position where you can edit. The former implies that it produces a sequence of positions (perhaps retaining additional state to be able to step forward/back efficiently). It also seems to me that it is useful to have a concept of a Position that is primarily optimized iteration. Perhaps EditingPosition is not the best of names. The basic idea is that we shouldn't need 2 different classes Position and PositionIterator and convert back-and-forth between them. I'm also still not clear on the proposed relation between EditingPosition and VisiblePosition. Does every EditingPosition have an associated VisiblePosition? How about vice versa? Is the mapping one-to-one? That is a good question - it depends how we want to treat stuff like :before/:after content, list items, and such. A renderer-based VisualPosition could technically select/iterate/identify such content, but wouldn't necessarily have a corresponding Editing/DOMPosition. OTOH, an EditingPosition probably would always have an associated VisiblePosition (At least off the top of my head I can't think of a case where the user can edit stuff that isn't rendered). An EditingPosition with reflection or somesuch could in theory have more than one VisiblePosition, but I think that's getting awfully fanciful. However, the immediate idea here is more one of code separation: to put the code that operates on renderers into its separate class. Safari doesn't make this non-contiguous region a selection. It marks it with a visual highlight. Only the first hit is actually selected. This makes a big difference when doing a find in editable text - typing only overtypes the first hit, rather than replacing the non-contiguous selection. Being able to represent a non-contiguous region is interesting, but it would be UI hell to allow such a thing to actually act as the user selection, and to get copied and pasted. FWIW Firefox seems to handle it fine, but I agree that it's probably not used much in practice (apart from column selection as you mentioned). I'm thinking more of feature-completeness vs. the HTML5 spec here, and the aforementioned non-selection highlights. But whether or not to add this should be a separate discussion. On Wed, Apr 7, 2010 at 3:53 AM, Justin Garcia justin.gar...@apple.com wrote: For example: divimg style=display:blockimg style=display:block/div [img1, 1] and [img2, 0] are different visually but would both be normalized to the same position under the above proposal. I think this is a great example and shows that normalizing [img, 0] to [parent-of-img, index-of-img] can probably only happen once styles and renderers are taken into account, which doesn't strictly contradict Darin's point though (?). I'm not sure how it implies that. Would you assign a real DOM position differently between the two styles? I think the real difference here is in affinity, i.e. whether the caret would be at the end of one line or the start of the next. However this doesn't affect the DOM parent and offset. Yes, that's what I meant - the DOM Position itself should obviously not be affected by styles/CSS. This however leads me to a clarification question of my own: The [img1, 1] in Justin's example would be an invalid position, no? Do you mean that (the internal representation of) [img/block, 0] and [img/block, 1] should render the caret at different positions (i.e., the start/left side and end/right side of the block line, respectively)? Or did you mean the case of 2 images (i.e., without a real internal visual position) on different lines? Thanks, - Roland ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Apr 7, 2010, at 1:59 AM, Roland Steiner wrote: On Wed, Apr 7, 2010 at 4:36 PM, Maciej Stachowiak m...@apple.com wrote: It's not clear to me how PositionIterator is the same concept as EditingPosition. The latter implies that it would only ever represent a position where you can edit. The former implies that it produces a sequence of positions (perhaps retaining additional state to be able to step forward/back efficiently). It also seems to me that it is useful to have a concept of a Position that is primarily optimized iteration. Perhaps EditingPosition is not the best of names. The basic idea is that we shouldn't need 2 different classes Position and PositionIterator and convert back-and-forth between them. I'm not sure I understand that goal. It's pretty normal for an iterator type to be distinct from the type of thing it iterates over. vectorT::iterator is not the same as T, even though it iterates over Ts. It is also not the same as vectorT itself. I'm not familiar with the exact state of the editing code, but that's what I think of when I hear a suggestion to combine PositionIterator and Position. I'm also not clear on where the name Editing comes from. What is it about this kind of position that's related to editing? Is it only allowed to represent positions where editing can happen? If so, then it can't be a full replacement for either Position or PositionIterator, since those are used to represent non-editable locations. If it can represent even non-editable locations, then I wonder how it differs from your proposed DOMPosition - would it be just the support for fast iteration? I'm also still not clear on the proposed relation between EditingPosition and VisiblePosition. Does every EditingPosition have an associated VisiblePosition? How about vice versa? Is the mapping one-to-one? That is a good question - it depends how we want to treat stuff like :before/:after content, list items, and such. A renderer-based VisualPosition could technically select/iterate/identify such content, but wouldn't necessarily have a corresponding Editing/ DOMPosition. OTOH, an EditingPosition probably would always have an associated VisiblePosition (At least off the top of my head I can't think of a case where the user can edit stuff that isn't rendered). An EditingPosition with reflection or somesuch could in theory have more than one VisiblePosition, but I think that's getting awfully fanciful. Could a VisiblePosition have more than one associated EditingPosition? However, the immediate idea here is more one of code separation: to put the code that operates on renderers into its separate class. I think to correctly iterate over positions where editing can happen, you need to be aware of rendering. For one thing, you want to skip stuff that is not rendered. Are you imagining that EditingPosition would be purely DOM based? Would EditingPosition or VisiblePosition be the type of object that represents a place where you can put the caret? I understand and sympathize with your attempt to clean up editing concepts, but I am not getting a sense of clean separation here. On Wed, Apr 7, 2010 at 3:53 AM, Justin Garcia justin.gar...@apple.com wrote: For example: divimg style=display:blockimg style=display:block/div [img1, 1] and [img2, 0] are different visually but would both be normalized to the same position under the above proposal. I think this is a great example and shows that normalizing [img, 0] to [parent-of-img, index-of-img] can probably only happen once styles and renderers are taken into account, which doesn't strictly contradict Darin's point though (?). I'm not sure how it implies that. Would you assign a real DOM position differently between the two styles? I think the real difference here is in affinity, i.e. whether the caret would be at the end of one line or the start of the next. However this doesn't affect the DOM parent and offset. Yes, that's what I meant - the DOM Position itself should obviously not be affected by styles/CSS. This however leads me to a clarification question of my own: The [img1, 1] in Justin's example would be an invalid position, no? Indeed; I believe he was trying to give an example of how such invalid positions might serve a meaningful purpose. Do you mean that (the internal representation of) [img/block, 0] and [img/block, 1] should render the caret at different positions (i.e., the start/left side and end/right side of the block line, respectively)? That is what happens currently, but I don't think that was Justin's main point. I think his point was that [img1, 1] renders at the end of the first line, and [img2, 0] renders at the start of the first line, even though both should normalize to a true DOM position of [div, 1] (between the two images). Or did you mean the case of 2 images (i.e., without a real internal
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Wed, Apr 7, 2010 at 6:19 PM, Maciej Stachowiak m...@apple.com wrote: I'm not sure I understand that goal. It's pretty normal for an iterator type to be distinct from the type of thing it iterates over. vectorT::iterator is not the same as T, even though it iterates over Ts. It is also not the same as vectorT itself. I'm not familiar with the exact state of the editing code, but that's what I think of when I hear a suggestion to combine PositionIterator and Position. I'm also not clear on where the name Editing comes from. What is it about this kind of position that's related to editing? Is it only allowed to represent positions where editing can happen? If so, then it can't be a full replacement for either Position or PositionIterator, since those are used to represent non-editable locations. If it can represent even non-editable locations, then I wonder how it differs from your proposed DOMPosition - would it be just the support for fast iteration? Yes, Editing seems a poor choice of term, let's stay with Position. I guess my proposal (which is actually not all that radical, I think) boils down to: 1.) remove explicit support for virtual positions from Position. 2.) PositionIterator seems to be very limited in usefulness and after step 1.) there doesn't seem to be a reason why Position itself couldn't be optimized for iteration and fast access the same way as PositionIterator is, removing the need for a separate PositionIterator class. 3.) If we do step 2.), then everything that requires an explicit (node, offset) interface requires a separate class. This class already exists in the form of RangeBoundaryPoint. RangeBoundaryPoint + those additional methods are what I termed DOMPosition. Ideally, creation of a DOMPosition would only be needed at the start and end of an editing operation as input and result for JS. 4.) Bundle all code from Position and VisualPosition that operates on renderers in VisualPosition and move code from VisualPosition that doesn't operate on renderers back into Position. The resulting VisualPosition class is based on renderers rather than nodes. Re-factor editing code that queries renderers to use VisualPosition objects instead. (perhaps naming the class RenderedPosition or somesuch might be clearer). Could a VisiblePosition have more than one associated EditingPosition? Right now I can't think of a circumstance where that could be the case (it would basically mean a given renderer is associated with more than one node). I think to correctly iterate over positions where editing can happen, you need to be aware of rendering. For one thing, you want to skip stuff that is not rendered. Are you imagining that EditingPosition would be purely DOM based? Would EditingPosition or VisiblePosition be the type of object that represents a place where you can put the caret? I understand and sympathize with your attempt to clean up editing concepts, but I am not getting a sense of clean separation here. ^_^; Perhaps that's because my proposal is not ambitious enough (and discussing these exact points was what I was hoping for with my initial mails): In the initial version at least I probably would want to change as little code as possible: DOMPosition as the bridge to JS, Position continuing to be used for most of the current editing/selection logic, VisualPosition as helper class, the interface to the renderers, etc. So I guess the answer to where to store the caret position would at first (continue to) be that VisiblePosition is used to discern the place where to put it, but Position is used to actually store it (if that makes sense). However, as you point out, much of the editing logic requires information from the renderers. Ideally this would mean changing most logic to work on renderers via VisiblePosition over time, leaving Position just as somewhat of a husk between DOMPosition and VisiblePosition. This would also allow us to neatly iterate over, and select, generated content (the question being if we could actually make use of that). However, with renderers getting destroyed and rebuilt constantly I'm not sure how far this is feasible. Cheers, - Roland ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
05.04.2010, в 22:46, Maciej Stachowiak написал(а): The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? Having fake positions like that is not good. I don't think there is a good reason for it. But the assumption got deeply embedded into the code, and it will take some doing to remove. One step in that direction would be to phase out all use of Position.deprecatedEditingOffset() and Position.node(). I think that one reason is performance - index-of-img can be costly. But I completely agree that we should try to get rid of these. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Apr 6, 2010, at 9:29 AM, Alexey Proskuryakov wrote: 05.04.2010, в 22:46, Maciej Stachowiak написал(а): The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? Having fake positions like that is not good. I don't think there is a good reason for it. But the assumption got deeply embedded into the code, and it will take some doing to remove. One step in that direction would be to phase out all use of Position.deprecatedEditingOffset() and Position.node(). I think that one reason is performance - index-of-img can be costly. But I completely agree that we should try to get rid of these. Let me elaborate on what Maciej and Alexey said. If we have an img element, there are three valid DOM positions adjacent to it: A: [ parent-of-img, child-offset-of-img ]: before the image B: [ img, 0 ]: inside the image C: [ parent-of-img, child-offset-of-img + 1 ]: after the image WebKit’s HTML editing code also makes use of an invalid DOM position: D: [ img, 1 ]: inside the image, but used to represent the position after the image Using D is never a good idea because it’s not a valid DOM position, so we can’t use it to construct a DOM range object. Having positions like D in our code at all leads to complexity and confusion. Using B to represent the position before the image is also unconventional; the position is clearly “inside the image element”, whatever that means. Given that, it seems that editing-related code that starts with a position such as B should quickly convert it to either A or C as appropriate. But computing A or C given a pointer to an image element is costly because it involves walking the parent's child nodes. In a large, flat document the cost is proportional to the size of the document. The same goes if we start with A or C and want to find the image element just before or just after the position. There are other issues when the document is being changed with positions extant. If we add a child element to the img element’s parent before the img element, both A and C end up pointing to a different place in the document, but B and D will still point where they did before. So if we convert editing code that currently uses a position like B to instead use a position like A, it’s easy to change behavior without realizing it. Encapsulating some of this into a class is one of the goals of the Position object, but we haven’t made much progress on untangling the mess. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
See my comments below. Thanks. – Ken On Apr 5, 2010, at 10:30 PM, Roland Steiner wrote: One additional question on position classes: The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? This is my fault. I made the initial error years ago to work with positions like [img, 0] - [img, 1] and [br,0] - [br, 1]. Though this is a mistake from the perspective of communicating positions outside of editing code, it is still very useful to know if a position is right before or after an image or br element. Some conveniences to ask such questions might be nice. If you wish to touch all the places in editing code where these degenerate positions are used, I have no reason to object. Cheers, - Roland On Fri, Apr 2, 2010 at 2:43 PM, Roland Steiner rolandstei...@google.com wrote: Hi all, As I am working on WebKit rich text editing these days, there are 2 issues that I would like to address. From a brief internal discussion both seem feasible and worthwhile, but since they involve changes to current code and behavior I wanted to ask the WebKit community in general, and the original authors of WebKit editing in particular, about your opinion: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). For my part, layout tests check behavior, they don't define it. They are meant to prevent regressions due to mistakes and unintended consequences, not to prevent genuine behavior improvements. .) The current Position classes are IMHO rather unfocused in their implementation, with lots of special cases and magical behavior, that still is often incorrect (e.g., with text that has padding, margins, or :before/:after content). For ease of further development they would therefore benefit from refactoring. The idea would be to change the classes into something along the lines of: DOMPosition: based on the current RangeBoundaryPoint, working on node + offset, interfacing with JavaScript EditingPosition (or TypeablePosition): based on the current PositionIterator for fast iteration, with most of the code of Position except for code that queries renderers VisiblePosition: change to work on renderers rather than nodes (moving such code from the current Position into this class). with explicit, but not implicit, conversion between them. Similarly for Ranges. In addition, a refactoring could add (or at least allow for) non-contiguous ranges and allow editing operations to work on arbitrary ranges/positions rather than just the (single) selection, which again currently is a pain point. In the long run I would envision to extend the code to allow multiple selections (such as for concurrent editing, or highlighting of find results, etc.), but that probably needs to be discussed separately. Yes, there is much magic in HTML editing. I always found it very difficult to make simple rules and adhere to them due to the complications of handling all the corner cases. So, it's hard for me to comment on your simply-stated proposals, since the implications of such design changes are vast. It's not clear that such a refactoring would make the code easier to change or maintain, and that seems to be a much more important goal than implementing any single feature (e.g. non-contiguous ranges). All that said, I am years removed from thinking about these problems on a regular basis. Mostly, I just wanted to own up to the position design mistake discussed at the top of the message. If only that were the greatest of my transgressions. It would be great if you could share your thoughts, Cheers, - Roland ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
One additional question on position classes: The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index-of-img(+1)] - round-tripping perhaps? Cheers, - Roland On Fri, Apr 2, 2010 at 2:43 PM, Roland Steiner rolandstei...@google.comwrote: Hi all, As I am working on WebKit rich text editing these days, there are 2 issues that I would like to address. From a brief internal discussion both seem feasible and worthwhile, but since they involve changes to current code and behavior I wanted to ask the WebKit community in general, and the original authors of WebKit editing in particular, about your opinion: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). .) The current Position classes are IMHO rather unfocused in their implementation, with lots of special cases and magical behavior, that still is often incorrect (e.g., with text that has padding, margins, or :before/:after content). For ease of further development they would therefore benefit from refactoring. The idea would be to change the classes into something along the lines of: DOMPosition: based on the current RangeBoundaryPoint, working on node + offset, interfacing with JavaScript EditingPosition (or TypeablePosition): based on the current PositionIterator for fast iteration, with most of the code of Position except for code that queries renderers VisiblePosition: change to work on renderers rather than nodes (moving such code from the current Position into this class). with explicit, but not implicit, conversion between them. Similarly for Ranges. In addition, a refactoring could add (or at least allow for) non-contiguous ranges and allow editing operations to work on arbitrary ranges/positions rather than just the (single) selection, which again currently is a pain point. In the long run I would envision to extend the code to allow multiple selections (such as for concurrent editing, or highlighting of find results, etc.), but that probably needs to be discussed separately. It would be great if you could share your thoughts, Cheers, - Roland * * ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Apr 5, 2010, at 10:30 PM, Roland Steiner wrote: One additional question on position classes: The current implementation allows for (and operates on) positions such as [img, 0] - [img, 1] or [br,0] - [br, 1]. Is there a fundamental reason to keep such positions within the internal representation rather than normalize them to [parent-of-img, index- of-img(+1)] - round-tripping perhaps? Having fake positions like that is not good. I don't think there is a good reason for it. But the assumption got deeply embedded into the code, and it will take some doing to remove. One step in that direction would be to phase out all use of Position.deprecatedEditingOffset() and Position.node(). Another possible step in the right direction: make rangeCompliantEquivalent a direct method on VisiblePosition instead of a free function, and start phasing out use of VisiblePositon.deepEquivalent() in favor of VisiblePosition.rangeCompliantEquivalent(). Eventually the internal representation can be changed. I am not sure offhand what other code uses the deep form of positions that may give positions at offset 0 or 1 from a void element. Regards, Maciej Cheers, - Roland On Fri, Apr 2, 2010 at 2:43 PM, Roland Steiner rolandstei...@google.com wrote: Hi all, As I am working on WebKit rich text editing these days, there are 2 issues that I would like to address. From a brief internal discussion both seem feasible and worthwhile, but since they involve changes to current code and behavior I wanted to ask the WebKit community in general, and the original authors of WebKit editing in particular, about your opinion: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). .) The current Position classes are IMHO rather unfocused in their implementation, with lots of special cases and magical behavior, that still is often incorrect (e.g., with text that has padding, margins, or :before/:after content). For ease of further development they would therefore benefit from refactoring. The idea would be to change the classes into something along the lines of: DOMPosition: based on the current RangeBoundaryPoint, working on node + offset, interfacing with JavaScript EditingPosition (or TypeablePosition): based on the current PositionIterator for fast iteration, with most of the code of Position except for code that queries renderers VisiblePosition: change to work on renderers rather than nodes (moving such code from the current Position into this class). with explicit, but not implicit, conversion between them. Similarly for Ranges. In addition, a refactoring could add (or at least allow for) non- contiguous ranges and allow editing operations to work on arbitrary ranges/positions rather than just the (single) selection, which again currently is a pain point. In the long run I would envision to extend the code to allow multiple selections (such as for concurrent editing, or highlighting of find results, etc.), but that probably needs to be discussed separately. It would be great if you could share your thoughts, Cheers, - Roland ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
On Apr 1, 2010, at 10:43 PM, Roland Steiner wrote: Hi all, As I am working on WebKit rich text editing these days, there are 2 issues that I would like to address. From a brief internal discussion both seem feasible and worthwhile, but since they involve changes to current code and behavior I wanted to ask the WebKit community in general, and the original authors of WebKit editing in particular, about your opinion: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). Which layout tests? Do they reference bugs? We can study the bugs to see if they truly ask for the behavior being tested. .) The current Position classes are IMHO rather unfocused in their implementation, with lots of special cases and magical behavior, that still is often incorrect (e.g., with text that has padding, margins, or :before/:after content). For ease of further development they would therefore benefit from refactoring. The idea would be to change the classes into something along the lines of: DOMPosition: based on the current RangeBoundaryPoint, working on node + offset, interfacing with JavaScript EditingPosition (or TypeablePosition): based on the current PositionIterator for fast iteration, with most of the code of Position except for code that queries renderers VisiblePosition: change to work on renderers rather than nodes (moving such code from the current Position into this class). with explicit, but not implicit, conversion between them. Similarly for Ranges. Hard to comment on this idea from such a high level view. I don't understand how EditingPosition is meant to be different from VisiblePosition. Is EditingPosition just a VisiblePosition that's also a place where you can edit? I don't understand how DOMPosition is different in intent from the current Position. I'm not sure you want the low-level class to based on RangeBoundaryPoint, since the latter has the ability to adjust in response to DOM changes, which I am not totally sure we want in this case. In addition, a refactoring could add (or at least allow for) non- contiguous ranges and allow editing operations to work on arbitrary ranges/positions rather than just the (single) selection, which again currently is a pain point. In the long run I would envision to extend the code to allow multiple selections (such as for concurrent editing, or highlighting of find results, etc.), but that probably needs to be discussed separately. Non-contiguous selections suck as UI, except for special cases like selecting tables by column. I don't think they are a good way to highlight find results. I don't think you want to end up with a non- contiguous selection highlighting all results when you do a find. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes
Hi all, As I am working on WebKit rich text editing these days, there are 2 issues that I would like to address. From a brief internal discussion both seem feasible and worthwhile, but since they involve changes to current code and behavior I wanted to ask the WebKit community in general, and the original authors of WebKit editing in particular, about your opinion: .) When a selection that starts in a table and ends outside it is deleted, the current code drags the adjacent outside content into the table. To me this is counter-intuitive (text can be dragged in, but not between cells, and not back outside), and it's also contrary to the behavior of other editors (FireFox, TextEdit, Word, etc.). The behavior is, however, enshrined in various layout tests, so I wonder if there was/is a reason to implement it this way. As this behavior also complicates fixing other bugs I wanted to see whether there would be much opposition to changing it (i.e., to content outside of a table staying outside on a delete operation). .) The current Position classes are IMHO rather unfocused in their implementation, with lots of special cases and magical behavior, that still is often incorrect (e.g., with text that has padding, margins, or :before/:after content). For ease of further development they would therefore benefit from refactoring. The idea would be to change the classes into something along the lines of: DOMPosition: based on the current RangeBoundaryPoint, working on node + offset, interfacing with JavaScript EditingPosition (or TypeablePosition): based on the current PositionIterator for fast iteration, with most of the code of Position except for code that queries renderers VisiblePosition: change to work on renderers rather than nodes (moving such code from the current Position into this class). with explicit, but not implicit, conversion between them. Similarly for Ranges. In addition, a refactoring could add (or at least allow for) non-contiguous ranges and allow editing operations to work on arbitrary ranges/positions rather than just the (single) selection, which again currently is a pain point. In the long run I would envision to extend the code to allow multiple selections (such as for concurrent editing, or highlighting of find results, etc.), but that probably needs to be discussed separately. It would be great if you could share your thoughts, Cheers, - Roland * * ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
