234640 - IHtmlSelectionObject & IHtmlTextRange implementation

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: ActiveX Wrapper, enhancement)

Description

[Alexandre Trémon]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.6) Gecko/20040113

implement these interfaces

Reproducible: Always
Steps to Reproduce:

Comment 1

[Alexandre Trémon]

Attachment: All IHtmlSelectionObject methods implemented

Comment 2

[Alexandre Trémon]

Attachment: All IHtmlSelectionObject methods implemented

Comment 3

[Alexandre Trémon]

Attachment: Some IHtmlTxtRange methods implemented

pasteHTML, get_text, parentElement are implemented

Comment 4

[Alexandre Trémon]

Attachment: Some IHtmlTxtRange methods implemented

pasteHTML, get_text, parentElement are implemented

Comment 5

[Alexandre Trémon]

Attachment: IHtmlDocument2::get_selection implementation

Comment 6

[Alexandre Trémon]

Attachment: Change in get_parentElement

Until now it was supposed that the CIEHtmlNode::mParent member was set at
creation time, as this creation happens in one of the 'PopulateXXX' method in
the CIEHtmlElementCollection class.
As an object can be created from the range, its parent (and its parent's
parent, etc.) is not necessarily set.

Comment 7

[Alexandre Trémon]

Some VB code to test this:

Dim htmlDoc As IHTMLDocument
Dim sel As IHTMLSelectionObject
Dim range As IHTMLTxtRange
Dim str As String
Dim elt As IHTMLElement

Set htmlDoc = MozillaBrowser.document
Set sel = htmlDoc.selection
'sel.Clear
'sel.empty
str = sel.Type
Set range = sel.createRange
Set elt = range.parentElement
Set elt = elt.parentElement
str = elt.tagName
str = range.Text
range.pasteHTML "<b>Hello world!</b>"

Comment 8

[Adam Lock]

Comment on attachment 141613 [details] [diff] [review]
IHtmlDocument2::get_selection implementation

In principle it looks fine, but I have some observations.

The CIEHtmlTxtRange::SetRange, CIEHtmlSelectionObject::SetSelection,
CIEHtmlSelectionObject::SetDOMDocumentRange methods should not take a smart
pointer - just pass a raw pointer.

CIEHtmlSelectionObject::createRange
does not free the memory allocated in call from get_type. Using CComBSTR would
fix this.

This functionality should also live in common/ not control. The
IEHTMLDocument.cpp is the exception because it has some browser specific code
in it, but the other parts of the DOM impl can be shared around

Comment 9

[Adam Lock]

Just a clarification. When I say "also live in common" I mean it should live in
common, and not in control at all. Other code such as the plugin picks up the
DOM implementation from the common so putting it there is ensures it can be used
by other code needing it.

I can't recall the reason IEHTMLDocument.cpp was left in control/ but it was
probably due to some control specific detail in its implementation.

Comment 10

[Alexandre Trémon]

IEHtmlSelection/IEHtmlTxtRange are in /common, as you can see in /common/makefile.
IEHtmlSelection.h/IEHtmlTxtRange .h are copied into /dist/include/ax_common so
it is possible to do #include "IEHtmlSelection.h" in IEHtmlDocument.cpp

Comment 11

[Alexandre Trémon]

Attachment: SetSelection & SetDOMDocumentRange take raw pointers

Comment 12

[Alexandre Trémon]

Attachment: CComBSTR in CreateRange

Comment 13

[Alexandre Trémon]

Attachment: call to SetDOMDocumentRange takes a raw pointer as parameter

Comment 14

[Alexandre Trémon]

Attachment: SetRange takes a raw pointer as parameter

Comment 15

[Alexandre Trémon]

Attachment: Added IHTMLControlRange

Comment 16

[Alexandre Trémon]

Attachment: Added IHTMLControlRange

Comment 17

[Alexandre Trémon]

Attachment: CreateRange now also creates IHTMLControlRange

Comment 18

[Alexandre Trémon]

I also implemented IHTMLControlRange::Collapse

Comment 19

[Adam Lock]

Comment on attachment 142221 [details] [diff] [review]
call to 
SetDOMDocumentRange takes a raw pointer as parameter

r=adamlock

Comment 20

[Johnny Stenback (:jst)]

Comment on attachment 142221 [details] [diff] [review]
call to 
SetDOMDocumentRange takes a raw pointer as parameter

sr=jst

Comment 21

[Adam Lock]

Comment on attachment 142221 [details] [diff] [review]
call to 
SetDOMDocumentRange takes a raw pointer as parameter

Seeking approval for 1.7b. Specific to the ActiveX control, low risk

Comment 22

[chris hofmann]

Comment on attachment 142221 [details] [diff] [review]
call to 
SetDOMDocumentRange takes a raw pointer as parameter

a=chofmann for 1.7b

Comment 23

[Adam Lock]

I've checked the range & selection objects into activex/common. I'll apply the
patches to compile & use them next (tomorrow some time).

Comment 24

[Adam Lock]

Fix is checked in

Comment 25

[Adam Lock]

Comment on attachment 141621 [details] [diff] [review]
Change in get_parentElement

r=adamlock

Ouch I forgot to check this one in. It will happen Friday night.

Comment 26

[Adam Lock]

Just checked in the missing diff.