633750 - deCOM nsIHTMLEditRules

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

nsIHTMLEditRules is completely internal to editor/libeditor/html and the concrete type is always known.

Comment 1

[Sebastian Kromp]

Attachment: Removed nsIHTMLEditRules and made the methods member of nsHTMLEditRules

This is my first patch with use of the queue, so I hope all went fine. I´m not sure about the "former nsIHTMLEditRules methods" comments, but I thought it could be useful.

Comment 2

[(no longer active)]

Comment on attachment 525061 [details] [diff] [review]
Removed nsIHTMLEditRules and made the methods member of nsHTMLEditRules

Please remove the "former nsIHTMLEditRules methods" comment.

But more importantly, you should remove all of the nsIHTMLEditRules uses from the tree.  I don't think that this patch even compiles.  Here is a full list of these uses: <http://mxr.mozilla.org/mozilla-central/ident?i=nsIHTMLEditRules>

Comment 3

[(no longer active)]

Specifically, you should just use static_cast to cast the mRules pointer to the nsHTMLEditRules concrete type instead of the query interface call.

Comment 4

[Sebastian Kromp]

Attachment: Removed nsIHTMLEditRules and made the methods member of nsHTMLEditRules

If I hadn´t messed up with the queue again, it should work fine now. I had took a look over it and couldn´t see something missing now.
I hope I´m right. :)

Comment 5

[(no longer active)]

Comment on attachment 527927 [details] [diff] [review]
Removed nsIHTMLEditRules and made the methods member of nsHTMLEditRules

Review of attachment 527927 [details] [diff] [review]:

This looks really good, thanks!

Have you also tested this on the try server?  If yes, it can land I think.

Comment 6

[Sebastian Kromp]

No I had now tried to, but it doesn´t worked. I have found this guide here https://wiki.mozilla.org/ReleaseEngineering/TryServer, but I´ve got everytime a error message from Putty: "No supported authentication methods available"
The last command at this site (with the -e ...) isn´t even recognized as valid command at all. I´m using TortoiseHG, maybe this is the problem? (And is it okay to ask here? Else I will ask per e-mail next time)

Anyway, thank you :)

Comment 7

[(no longer active)]

I've never used putty with tortoisehg myself (I use the hg command from the terminal!), but looking at http://row1.info/news/91-blog/265-using-tortoisehg-with-bitbucket-and-ssh.html, it seems that you need a line like this in your mercurial.ini file:

ssh = "C:\Program Files (x86)\TortoiseHg\TortoisePlink.exe" -ssh -2 -i C:\Users\YourUsername\.ssh\yourkey.ppk

Also, make sure that your LDAP user name in the URL.

If that doesn't work out, you can always use the command line.  In order to do that, you should export your SSH key from putty into the OpenSSH format, put it in ~/.ssh/.id_rsa (private key) and ~/.ssh/.id_rsa.pub (public key), and modify ~/.ssh/config like this: <https://developer.mozilla.org/en/Creating_Mercurial_User_Repositories#Configuring_ssh>, and push to try using hg push -f ssh://hg.mozilla.org/try.  That configuration is guaranteed to work.  :-)

Let me know if you can't get it to work.

Comment 8

[Sebastian Kromp]

Okay, I`ve now got the testresults:
http://tbpl.mozilla.org/?tree=Try&rev=5eb262e4e6e3
There are some warnings and errors, but I´m not sure if they have something to do with my changes.

Comment 9

[Josh Matthews [:jdm]]

The non-android reftests are a known intermittent orange. Jetpack failures are expected, I believe. Android browser-chrome failures are expected. Android Mochitest 2 looks like an infra or harness failure. Android R2 looks like the same. The mobile folks tell me that the failing Android R1 tests are nothing to worry about either. I think you're clear!

Comment 10

[(no longer active)]

Indeed!  Thanks for your patch!  :-)

Comment 11

[Mounir Lamouri (:mounir)]

Thanks for the patch Sebastian. It's now pushed to the cedar repository which will be merged to mozilla-central soon.

Comment 12

[Sebastian Kromp]

Thank you very much! I´m glad that I could help. :)

Comment 13

[Mounir Lamouri (:mounir)]

It's now in mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/33d37164fd18

Comment 14

[Eric Shepherd [:sheppy]]

Noted the removal of this interface.