850213 - Update nsHTMLInputElement.cpp to use nsIContentPrefService2

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

Use the new API added in bug 699859

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: nsHTMLInputElement

Olli: this changes the contentPrefs usage in nsHTMLInputElement (which handles the per-uri folder chosen for input type=file) to the new async API nsIContentPrefService2.

The bulk of the change was to change the FetchLastUsedDirectory function to no longer return a folder, but to retrieve the folder async'ly and also show the file picker through the callback.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 729873 [details] [diff] [review]
nsHTMLInputElement

># HG changeset patch
># Parent 4d3250f3afea022bd5779912c208fb188ec089f1
># User Felipe Gomes <[e-mail]>
>
>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>--- a/content/html/content/src/nsHTMLInputElement.cpp
>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>@@ -251,16 +251,60 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsHTMLInpu
> nsHTMLInputElement::nsFilePickerShownCallback::nsFilePickerShownCallback(
>   nsHTMLInputElement* aInput, nsIFilePicker* aFilePicker, bool aMulti)
>   : mFilePicker(aFilePicker)
>   , mInput(aInput)
>   , mMulti(aMulti)
> {
> }
> 
>+NS_IMPL_ISUPPORTS1(UploadLastDir::ContentPrefCallback, nsIContentPrefCallback2)
>+
>+NS_IMETHODIMP
>+UploadLastDir::ContentPrefCallback::HandleCompletion(uint16_t reason)
s/reason/aReason/

>+{
>+  nsCOMPtr<nsIFile> localFile = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID);
>+  if (!localFile)
>+    return NS_ERROR_OUT_OF_MEMORY;
coding style would be
if (expr) {
  stmt;
}
but since the failure happens more likely during shutdown, 
NS_ENSURE_STATE(localFile): should be ok

>+  } else {
>+    nsString prefStr;
nsAutoString

>+UploadLastDir::ContentPrefCallback::HandleResult(nsIContentPref* pref)
aPref

>+{
>+  NS_IF_ADDREF(pref);
>+  mResult = pref;
Er, you're leaking pref here.
mResult is nsCOMPtr so when you assign to it, the value gets addrefed.
So, drop NS_IF_ADDREF(pref);

>+NS_IMETHODIMP
>+UploadLastDir::ContentPrefCallback::HandleError(nsresult error)
aError

>+  class ContentPrefCallback MOZ_FINAL
>+      : public nsIContentPrefCallback2
>+    {
>+      public:
>+      ContentPrefCallback(nsIFilePicker* filePicker, nsIFilePickerShownCallback* fpCallback)
aFilePicker, aFpCallback

>+      : mFilePicker(filePicker)
>+      , mFpCallback(fpCallback)
>+      { }
>+
>+      virtual ~ContentPrefCallback()
>+      { }
>+
>+      NS_DECL_ISUPPORTS
>+      NS_DECL_NSICONTENTPREFCALLBACK2
>+
>+      nsCOMPtr<nsIFilePicker> mFilePicker;
>+      nsCOMPtr<nsIFilePickerShownCallback> mFpCallback;
>+      nsCOMPtr<nsIContentPref> mResult;
>+    };
> };
 { should be below 'c' of the 'class'

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Attachment: Test fix for input element

This test needed to wait some more for the file picker to show to satisfy the showing/not showing conditions.
The test was already convoluted with a lot of executeSoon waiting on things, so I grabbed a waitForCondition from a head.js somewhere and rewrote it using yield.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 730876 [details] [diff] [review]
Test fix for input element

> /** Test for Bug 36619 **/
> 
>+let gGen = doTest();
>+function continueTest() {
>+  gGen.next();
>+}
>+
>+function waitForCondition(condition, errorMsg) {
>+  var tries = 0;
>+  var interval = setInterval(function() {
>+    if (tries >= 10) {
>+      ok(false, errorMsg);
>+      moveOn();
>+    }
>+    if (condition()) {
>+      moveOn();
>+    }
>+    tries++;
>+  }, 100);
>+  var moveOn = function() { clearInterval(interval);
clearInterval should be in the next line

Comment 5

[:Felipe Gomes (needinfo for replies!)]

when I originally filed the bug I thought the HTMLInputElement and nsEditorSpellCheck changes would be related, but they turned out not to be so it's probably better to do that in a different bug

Comment 6

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c82d8a79e16e

Comment 7

[Drew Willcoxon :adw]

(In reply to :Felipe Gomes from comment #5)
> when I originally filed the bug I thought the HTMLInputElement and
> nsEditorSpellCheck changes would be related, but they turned out not to be
> so it's probably better to do that in a different bug

bug 856270

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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