Closed Bug 573672 Opened 10 years ago Closed 10 years ago

Use getDefaultFileName in an e10s-safe way

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
We currently pass the browser.contentDocument into getDefaultFileName - Bad for e10s. Internally, getDefaultFileName just uses the document.title property to create a nice filename.

We can do the same thing by passing .contentTitle is as a possible default and then apply the same regex.
Attachment #452977 - Flags: review?(21)
Comment on attachment 452977 [details] [diff] [review]
patch

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js
>     let browser = Browser.selectedBrowser;
>-    let fileName = getDefaultFileName(null, browser.documentURI, browser.contentDocument, null);
>+    let fileName = getDefaultFileName(browser.contentTitle, browser.documentURI, null, null);
>+    fileName = fileName.replace(/^\s+|\s+$/g, "");
> #ifdef MOZ_PLATFORM_MAEMO

Maybe a little comment explaining why we're doing the regexp after could be useful, not sure others will understand otherwise.
Attachment #452977 - Flags: review?(21) → review+
pushed with comments:
http://hg.mozilla.org/mobile-browser/rev/e9f1b9d688e3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #1)
> >+    fileName = fileName.replace(/^\s+|\s+$/g, "");

fileName.trim() ?
(In reply to comment #3)
> (In reply to comment #1)
> > >+    fileName = fileName.replace(/^\s+|\s+$/g, "");
> 
> fileName.trim() ?

Yeah. I was just blindly copynig what getDefaultFileName does.

I'll fix this in an upcoming checkin
bugspam
Assignee: nobody → mark.finkle
You need to log in before you can comment on or make changes to this bug.