A small fix to the text editor selection getSelected function #1

Closed
wants to merge 4 commits into from

2 participants

@ironymark

I have been experimenting with the YUI 3 RTE bidi feature and I have made two changes that I would like added to yui:
1 - wired in bidi to the sandbox editor example.
2 - fixed a bug in Selection.getSelected(). The bug was selecting all elements that had a font face in addition to elements in the current selection.

This is a simple fix to resolve this but I think the function really needs to be rewritten so that it does not rely on execCommand ('fontname'..) to mark selected elements.

@davglass

The source in the repo should not be loading YUI from the CDN, it should always be loading it locally as the local code is always the latest version. Besides 3.2.0 is the lastest, not pr1.

That is what this patch does - commented out the call to load YUI from the CDN.

YUI Library member

Sorry, was looking at the wrong file.. I don't develop on that file anymore, so I haven't looked at it.. ;)

@davglass
YUI Library member

Adil -- We don't accept pull requests from GitHub.

You need to submit a pull request from our site: http://yuilibrary.com/contrib/#pullrequest
As well as sign and return a CLA before any code can be accepted.

As for the "rewrite" of the function, you are more than welcome to attempt it. The issue is that the execCommand() method is a "strange" beast. This method doesn't attempt to get the selected elements, it attempts to get "the list of what execCommand would do if executed on this selection" list of elements. That's a totally different list of things.

For example, take this markup:

<p>This is a test.</p><ul><li>foo</li><li>bar</li></ul><p>This is another test</p>

Now, if you have "test.</p><ul><li>foo</li><li>bar</li></ul><p>This is another" selected, a normal selection handler will return: "P, UL, P" which is totally incorrect. Using the execCommand method, you will get something like this: "span, span, span, span, span" and the markup would change to this:

<p>This is a <span>test.</span></p><ul><li><span>foo</span></li><li><span>bar</span></li></ul><p><span>This is another</span> test</p>

Those are actually the elements that you are wanting to work with, so those are the elements that are being returned.

Does that make sense?

@tripp tripp referenced this pull request in tripp/yui3 Jan 25, 2012
@lsmith lsmith Decouple column parsing, +nested cols
The parsing of column array data was in Core,
but was parsed into three different forms:
1) for column access from Core's get('columns.<key>')
2) for column prep for header rendering
3) for column prep for body rendering

Now the columns are passed to the views to do with as
they will, and Core only does what is needed for #1.

Also, parsing nested headers efficiently is a pain in
the ass.  The 'children' config doesn't allow implementers
to choose which cells get rowspan when the number of
header rows is greater than the number of column descendant
levels.  As is done in 3.4.1, parent cells are always
rowspan 1.  I prefer to have data related columns have
rowspan 1, but the logic to support that with the "children"
config would be crazy complex.  Implementers can move stuff
around in `table.head.columns` before `render()` if they want
different cell placement and spanning.
d151341
@tripp tripp referenced this pull request in tripp/yui3 Jan 25, 2012
@lsmith lsmith Review feedback round #1
1. Fix typo in TFOOT_TEMPLATE
2. Don't store the array data populating the `data` ModelList in the attribute
3. Other minor typos
9f7cb9c
@ericf ericf referenced this pull request Nov 1, 2012
Merged

Async faster with Y.soon! #304

@drjayvee drjayvee referenced this pull request Aug 2, 2013
Open

Various issues with Button #973

3 of 5 tasks complete
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment