Open Bug 1042205 Opened 6 years ago Updated 5 years ago

Provide search suggestions from our history db on error pages

Categories

(Firefox for Android :: General, defect)

x86_64
Linux
defect
Not set

Tracking

()

People

(Reporter: wesj, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Bug 1042201 is about providing search suggestions from search providers on error pages, but we can also query our own history for pages that have similar urls to the one that was typed. i.e. we could fix amazpn.com to amazon.com for the user. AND we get to do that without leaking any private data. Huzzah!
Blocks: 940453
Attached patch Patch (obsolete) — Splinter Review
Attached patch PatchSplinter Review
Revamped for the new world. This is the patch from bug 1042201 with the search provider stuff removed because of reasons listed there.
Attachment #8460609 - Attachment is obsolete: true
Attachment #8477683 - Flags: review?(mark.finkle)
Attached patch PatchSplinter Review
Forgot the strings changes in the other patch.
Attachment #8477689 - Flags: review?(mark.finkle)
I'll get to these soon. Looking at some other Fx33 stuff right now.
Comment on attachment 8477683 [details] [diff] [review]
Patch


>diff --git a/mobile/android/modules/NetErrorHelper.jsm b/mobile/android/modules/NetErrorHelper.jsm

>+handlers.suggestions = {

>+  onPageShown: function(browser) {

>+    this._getFromBrowserDB(node, doc.location.href);

nit: _get is a bit off here since we are returning nothing. How about _updateFromDB or just _update ?

>+  _getFromBrowserDB: function(node, url) {

>+    var self = this;

let

you could pass in | this | and impl the methods in your helper, if you want to avoid the | self |

>+    let dbFile = FileUtils.getFile("ProfD", ["browser.db"]);
>+    return Services.storage.openDatabase(dbFile);

How safe is this? Java DB accessed from JS. Might be OK since it's read-only, but the SQLite versions could be different.

>diff --git a/mobile/android/themes/core/netError.css b/mobile/android/themes/core/netError.css

Add a screenshot here and let's get UX to help with making it look pretty.
Attachment #8477683 - Flags: review?(mark.finkle) → feedback+
Comment on attachment 8477689 [details] [diff] [review]
Patch

>diff --git a/mobile/android/chrome/content/netError.xhtml b/mobile/android/chrome/content/netError.xhtml

>       <div id="errorDescriptionsContainer">
>         <div id="ed_generic">&generic.longDesc;</div>
>-        <div id="ed_dnsNotFound">&dnsNotFound.longDesc3;</div>
>+        <div id="ed_dnsNotFound">&dnsNotFound.longDesc4;</div>
>         <div id="ed_fileNotFound">&fileNotFound.longDesc;</div>
>-        <div id="ed_malformedURI">&malformedURI.longDesc;</div>
>+        <div id="ed_malformedURI">&malformedURI.longDesc2;</div>


>diff --git a/mobile/locales/en-US/overrides/netError.dtd b/mobile/locales/en-US/overrides/netError.dtd

>-<!ENTITY dnsNotFound.longDesc3 "
>+<!ENTITY dnsNotFound.longDesc4 "

Make sure you cover all the possible issues, like you did with the wifi patch

>   <li>Check the address for typing errors such as
>     <strong>ww</strong>.example.com instead of
>     <strong>www</strong>.example.com
>+    <ul id='suggestions'></ul></li>

I feel like we need a "Did you mean?" type of header. A screenshot will help direct the design.
Attachment #8477689 - Flags: review?(mark.finkle) → feedback+
You need to log in before you can comment on or make changes to this bug.