Last Comment Bug 633750 - deCOM nsIHTMLEditRules
: deCOM nsIHTMLEditRules
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Sebastian Kromp
:
Mentors:
Depends on:
Blocks: deCOM
  Show dependency treegraph
 
Reported: 2011-02-12 10:09 PST by Kyle Huey [:khuey] (khuey@mozilla.com)
Modified: 2011-06-16 13:28 PDT (History)
8 users (show)
mounir: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Removed nsIHTMLEditRules and made the methods member of nsHTMLEditRules (10.70 KB, patch)
2011-04-11 06:01 PDT, Sebastian Kromp
ehsan: review-
Details | Diff | Splinter Review
Removed nsIHTMLEditRules and made the methods member of nsHTMLEditRules (15.47 KB, patch)
2011-04-23 04:07 PDT, Sebastian Kromp
ehsan: review+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] (khuey@mozilla.com) 2011-02-12 10:09:26 PST
nsIHTMLEditRules is completely internal to editor/libeditor/html and the concrete type is always known.
Comment 1 Sebastian Kromp 2011-04-11 06:01:42 PDT
Created attachment 525061 [details] [diff] [review]
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 :Ehsan Akhgari 2011-04-11 10:48:43 PDT
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 :Ehsan Akhgari 2011-04-11 10:50:31 PDT
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 2011-04-23 04:07:39 PDT
Created attachment 527927 [details] [diff] [review]
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 :Ehsan Akhgari 2011-04-25 13:02:39 PDT
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 2011-04-25 15:01:27 PDT
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 :Ehsan Akhgari 2011-04-28 08:36:08 PDT
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 2011-05-05 07:36:35 PDT
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] 2011-05-05 14:18:05 PDT
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 :Ehsan Akhgari 2011-05-06 14:45:45 PDT
Indeed!  Thanks for your patch!  :-)
Comment 11 Mounir Lamouri (:mounir) 2011-05-07 01:11:57 PDT
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 2011-05-07 02:56:29 PDT
Thank you very much! I´m glad that I could help. :)
Comment 13 Mounir Lamouri (:mounir) 2011-05-08 09:30:10 PDT
It's now in mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/33d37164fd18
Comment 14 Eric Shepherd [:sheppy] 2011-06-16 13:28:43 PDT
Noted the removal of this interface.

Note You need to log in before you can comment on or make changes to this bug.