IHtmlElement::insertAdjacentHtml & put_id implementations

RESOLVED FIXED

Status

Core Graveyard
Embedding: ActiveX Wrapper
--
enhancement
RESOLVED FIXED
14 years ago
6 years ago

People

(Reporter: Alexandre Trémon, Assigned: Alexandre Trémon)

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040616
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040616

IHtmlElement::insertAdjacentHtml & put_id implementations are not yet provided
in CIEHtmlElement

Reproducible: Always
Steps to Reproduce:
(Assignee)

Updated

14 years ago
Assignee: adamlock → atremon
(Assignee)

Comment 1

14 years ago
Created attachment 151128 [details] [diff] [review]
IHtmlElement::insertAdjacentHtml & put_id implementations

IHtmlElement::insertAdjacentHtml & put_id implementations in CIEHtmlElement
(Assignee)

Updated

14 years ago
Attachment #151128 - Flags: review?(adamlock)

Comment 2

14 years ago
Comment on attachment 151128 [details] [diff] [review]
IHtmlElement::insertAdjacentHtml & put_id implementations

insertAdjacentHTML needs a USES_CONVERSION macro and OLE2CW instead of OLE2W.

Otherwise r=adamlock
Attachment #151128 - Flags: review?(adamlock) → review+
(Assignee)

Comment 3

14 years ago
Created attachment 155912 [details] [diff] [review]
Uses_Conversion & OLE2CW
(Assignee)

Updated

14 years ago
Attachment #151128 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #155912 - Flags: superreview?(jst)
Comment on attachment 155912 [details] [diff] [review]
Uses_Conversion & OLE2CW

- In CIEHtmlElement::put_id():

+    nsAutoString strID(OLE2CW(v));

Any reason not to use nsDependentString here to avoid a string copy and
possibly a malloc/free?

- In CIEHtmlElement::get_id():

+	 return SUCCEEDED(rv)?S_OK: E_FAIL;

Maybe be more consistent with spacing around the operators there (and in all
other occurances), how about:

+	 return SUCCEEDED(rv) ? S_OK : E_FAIL;

sr=jst
Attachment #155912 - Flags: superreview?(jst) → superreview+
Forgot to mention that the nsAutoString in CIEHtmlElement::insertAdjacentHTML()
could be an nsDependentString too, unless I missed something...
(Assignee)

Comment 6

14 years ago
(In reply to comment #5)
> Forgot to mention that the nsAutoString in CIEHtmlElement::insertAdjacentHTML()
> could be an nsDependentString too, unless I missed something...

Thanks for reviewing, Johnny!
nsAutoString makes a copy of the buffer, unlike nsDependentString, is that it?
Even with the string guide, I don't know much of the subtelties of Mozilla's
strings yet ... :-)
(Assignee)

Comment 7

14 years ago
Created attachment 157612 [details] [diff] [review]
following Johnny's comments
Attachment #155912 - Attachment is obsolete: true
(Assignee)

Comment 8

14 years ago
Adam,

Can you check in this patch for me?

Thanks!

Comment 9

13 years ago
Fix checked in with one minor modification to add a missing #include
"nsIDOMHtmlElement.h" that may have been included by something else originally
but isn't anymore.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 10

12 years ago
How does one use insertAdjacentHTML? If I go some place such as:

  http://webfx.eae.net/bombProject/bomberman.html

I get this on the js console:

  Error: document.body.insertAdjacentHTML is not a function
  Source File: http://webfx.eae.net/bombProject/bomberman.html
  Line: 39
(Assignee)

Comment 11

12 years ago
> How does one use insertAdjacentHTML? If I go some place such as:
> 
>   http://webfx.eae.net/bombProject/bomberman.html
> 

I suppose you use your browser on that page. InserAdjacentHtml is not implemented in seamonkey/firefox.
This here is about the ActiveX control, a bridge that can be used by apps where you want to replace the ShellDocView control (IE) by Gecko.
Component: Embedding: ActiveX Wrapper → Embedding: ActiveX Wrapper
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.