693933 - nsFrameSelection::GetSelection() should return nsTypedSelection

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[alexander :surkov (:asurkov)]

spun off bug 688126 comment #5:
> 2) Can nsTypedSelection be exposed rather than nsISelection (for example,
> change existing nsFrameSelection::GetSelection or add new method)?
> nsTypedSelection allows to deal with nsIRange directly.

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: patch

smaug this is about the simplest patch I can write for this I just cut / pasted nsTypedSelection from nsSelection.cpp into a new header of its own and hit things with a hammer a little to make it build.  If you'd like some or a lot of stuff to be cleaned up while working on this I might be able to try although I may not have time for a week or two.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 619487 [details] [diff] [review]
patch

Review of attachment 619487 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsTypedSelection.h
@@ +3,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef NSTYPEDSELECTION_H_

nsTypedSelection_h

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to Ms2ger from comment #2)
> Comment on attachment 619487 [details] [diff] [review]
> patch
> 
> Review of attachment 619487 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/generic/nsTypedSelection.h
> @@ +3,5 @@
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> > + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> > + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> > +
> > +#ifndef NSTYPEDSELECTION_H_
> 
> nsTypedSelection_h

assuming that's the pattern in layout land fixed locally.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Should nsTypedSelection.h include nsRange.h?  When I try to include it elsewhere, I get a compile error until I include nsRange.h too.

Comment 5

[alexander :surkov (:asurkov)]

btw, Trevor, are going to land it?

Comment 6

[Trevor Saunders (:tbsaunde)]

so, I was about to finally land this, but you'll see in this try run https://tbpl.mozilla.org/?tree=Try&rev=70a2a9aa8088  I somehow managed to break a ref test on osx 10.5 only, any ideas?  I'll probably push this to try again and hope its the fault of something else that got backed out or fixed if nobody has a good idea.

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Are you sure that's related to this bug?  I've done a bunch of try runs with your patch, and that test passed on them, e.g.:

https://tbpl.mozilla.org/?tree=Try&rev=5b058bd6f2ef

Comment 8

[Trevor Saunders (:tbsaunde)]

ok then, I pushed it as https://hg.mozilla.org/integration/mozilla-inbound/rev/b7b7c790cc5e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b7b7c790cc5e