Closed Bug 522668 Opened 15 years ago Closed 8 years ago

Integrate PlacesDBUtils.checkAndFixDatabase() in about:support

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed

People

(Reporter: mak, Assigned: gasolin, Mentored)

References

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 file)

Something we can't do for 3.6 due to string freeze, but cool for next version.
Do we still want this?
Mentor: adw
Flags: needinfo?(mak77)
I think it might still be worth it, as it is now I must ask users to install and use my add-on to check database integrity. It's not a priority and PlacesDBUtils needs a rewrite (task, promises, Sqlite.jsm and such) for which there's a separate bug.
Flags: needinfo?(mak77)
Whiteboard: [good first bug][lang=js]
Hi, I would like to take this up. Could I have a heads up on how to proceed ?

Thanks,
Shoubhik
Flags: needinfo?(adw)
Hi Shoubhik, sure, thanks for volunteering!  Have you gotten the Firefox source and tried building it?  First you'll need to do that, and these pages explain how:

https://developer.mozilla.org/en-US/docs/Introduction
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide

I'm going to describe how to fix this bug.  It might sound like a lot of work, but actually it should be pretty simple.  Let me know here in the bug when you have questions.  You can also join #fx-team on IRC to talk to me and other Firefox developers.  I'm adw there.  https://wiki.mozilla.org/IRC

You'll need to modify the about:support page.  It's written in XHTML and lives here: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.xhtml

You can start by adding a new "major-section" to aboutsupport.xhtml between "Important Locked Preferences" and "JavaScript", with a title of "Places Database".  It should have a table, just like the table in the "Application Basics" section.  The table should have one row.  The first column should say "Integrity", and the second column should have a button named "Verify Integrity".

You can't just put English (or any other language) strings direcly in the XHTML.  Instead we use separate files that store UI strings for different languages, and then in your JS and HTML, you reference those strings.  For example, in aboutsupport.xhtml, this line says to use the refreshProfile.title string inside the <h3>: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.xhtml?rev=782630ecde30#33

The about:support strings live here: http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/aboutSupport.dtd  So you would define your three new strings and then reference them in the XHTML.

Once you finish the XHTML part, you can move on to the JavaScript part.  The JS file that controls the page lives here: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js

The PlacesDBUtils.checkAndFixDatabase() function that Marco is talking about lives here, in PlacesDBUtils.jsm: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/PlacesDBUtils.jsm?rev=ad5327aa82fd#112

"jsm" means JavaScript module.  You'll need to lazily import that module in aboutsupport.js to use it, the same way aboutsupport.js already imports PluralForm.jsm here: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js?rev=782630ecde30#14  Use the URL "resource://gre/modules/PlacesDBUtils.jsm".

When the user clicks the Verify Integrity button, you want to call checkAndFixDatabase().  You can look at the other buttons in the Application Basics section to see how they're hooked up.

checkAndFixDatabase() has a callback argument, a function that's called when the database has been checked and fixed.  That function is passed an array of log messages.  I think it would be nice to show those messages to the user somehow.  I think underneath the Verify Integrity button, but in the same row, there should be a <pre> that is hidden until the callback function is called.  When the callback is called, you put all the log messages in the <pre> and then show it.  There may be many messages, so the <pre> should have a CSS style with a small height and an `overflow: auto` so that it shows some scrollbars to allow the user to scroll through the messages.  Add that CSS to this file: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/aboutSupport.css

Good luck!
Flags: needinfo?(adw)
(In reply to Drew Willcoxon :adw from comment #4)
> Hi Shoubhik, sure, thanks for volunteering!  Have you gotten the Firefox
> source and tried building it?  First you'll need to do that, and these pages
> explain how:
> 
> https://developer.mozilla.org/en-US/docs/Introduction
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide
> 
> I'm going to describe how to fix this bug.  It might sound like a lot of
> work, but actually it should be pretty simple.  Let me know here in the bug
> when you have questions.  You can also join #fx-team on IRC to talk to me
> and other Firefox developers.  I'm adw there.  https://wiki.mozilla.org/IRC
> 
> You'll need to modify the about:support page.  It's written in XHTML and
> lives here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.
> xhtml
> 
> You can start by adding a new "major-section" to aboutsupport.xhtml between
> "Important Locked Preferences" and "JavaScript", with a title of "Places
> Database".  It should have a table, just like the table in the "Application
> Basics" section.  The table should have one row.  The first column should
> say "Integrity", and the second column should have a button named "Verify
> Integrity".
> 
> You can't just put English (or any other language) strings direcly in the
> XHTML.  Instead we use separate files that store UI strings for different
> languages, and then in your JS and HTML, you reference those strings.  For
> example, in aboutsupport.xhtml, this line says to use the
> refreshProfile.title string inside the <h3>:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.
> xhtml?rev=782630ecde30#33
> 
> The about:support strings live here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/
> global/aboutSupport.dtd  So you would define your three new strings and then
> reference them in the XHTML.
> 
> Once you finish the XHTML part, you can move on to the JavaScript part.  The
> JS file that controls the page lives here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js
> 
> The PlacesDBUtils.checkAndFixDatabase() function that Marco is talking about
> lives here, in PlacesDBUtils.jsm:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/
> PlacesDBUtils.jsm?rev=ad5327aa82fd#112
> 
> "jsm" means JavaScript module.  You'll need to lazily import that module in
> aboutsupport.js to use it, the same way aboutsupport.js already imports
> PluralForm.jsm here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.
> js?rev=782630ecde30#14  Use the URL
> "resource://gre/modules/PlacesDBUtils.jsm".
> 
> When the user clicks the Verify Integrity button, you want to call
> checkAndFixDatabase().  You can look at the other buttons in the Application
> Basics section to see how they're hooked up.
> 
> checkAndFixDatabase() has a callback argument, a function that's called when
> the database has been checked and fixed.  That function is passed an array
> of log messages.  I think it would be nice to show those messages to the
> user somehow.  I think underneath the Verify Integrity button, but in the
> same row, there should be a <pre> that is hidden until the callback function
> is called.  When the callback is called, you put all the log messages in the
> <pre> and then show it.  There may be many messages, so the <pre> should
> have a CSS style with a small height and an `overflow: auto` so that it
> shows some scrollbars to allow the user to scroll through the messages.  Add
> that CSS to this file:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/
> aboutSupport.css
> 
> Good luck!

Thanks for the awesome explanation.

I'm done with the following at the moment :
- Added the UI components in the about:support page
- Add the event handler to the button.

Everything looks good till here.


The event handler for the "Verify Integrity" button calls the checkAndFixDatabase() method. I've passed an anonymous method callback as the lone argument. 

The anonymous method ( for now ) has a console.log("..") statement which doesn't seem to be appearing the browser console. 

This makes me slightly uncomfortable because I'm unable to figure out if checkAndFixDatabase() worked.

A couple of questions:

- checkAndFixDatabase() has an optional argument for mentioning scope. Could you please tell me the significance of that ?
- Apart from logs array being passed on to the callback, is there any other place where I could check out application logs Or more specifically how checkAndFixDatabase() fared.
Flags: needinfo?(adw)
Cool, glad you're making progress.

(In reply to sbose78 from comment #5)
> - checkAndFixDatabase() has an optional argument for mentioning scope. Could
> you please tell me the significance of that ?

Sure, it's used as the `this`, or the receiver, of the callback invocation.  In other words, if you happen to have an object:

foo = {
  bar: "hey",
  myCallback: function () {
    dump("myCallback this.bar=" + this.bar + "\n");
  }
};

Then you can call checkAndFixDatabase like this:

  PlacesDBUtils.checkAndFixDatabase(foo.myCallback, foo);

And then inside myCallback, `this` will be foo.  You can see where PlacesDBUtils calls your callback here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/PlacesDBUtils.jsm?rev=ad5327aa82fd#66

These days we don't really need this kind of argument anymore because we have the JS bind() function: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind

So we would probably write this instead:

  PlacesDBUtils.checkAndFixDatabase(foo.myCallback.bind(foo));

Anyhow, bind() and the aScope argument probably aren't useful for your particular patch.

> - Apart from logs array being passed on to the callback, is there any other
> place where I could check out application logs Or more specifically how
> checkAndFixDatabase() fared.

console.log should work inside aboutSupport.js.  It does for me.  If you're calling it from PlacesDBUtils.jsm, though, that probably won't work.

I don't know off-hand why it doesn't work for you.  There may be some preference you have to set that I set a long time ago, or maybe it's because I'm using a debug build.

You can try using dump() instead.  That's actually what I use most of the time.  You have to open about:config and create a new boolean preference called browser.dom.window.dump.enabled and set it to true.  More info: https://developer.mozilla.org/en-US/docs/Web/API/Window.dump

When all else fails, you can also try a window.alert().
Flags: needinfo?(adw)
One thing you might try is using the web console within about:support instead of using the browser console.  I see console messages from aboutSupport.js in both, but if you're not seeing them in the browser console, maybe the web console will show them.
Thanks a lot !
I could see the error and fix my code. I'll make the DOM changes to show the callback logs and let you know how things go. 

As a heads up, could you please tell me the process I should be following to have you review my code?

Thanks
Shoubhik
Flags: needinfo?(adw)
You'll post your patch here in the bug using the "Add an attachment" link above.  On that page, there's a "Flags:" section.  If your patch is finished, click the "review" dropdown, select the question mark (?), put my email address in the Requestee box (or type :adw, and then choose my email address in the autocomplete), and then click the Submit button at the bottom of the page.  Type some comments too if you'd like.

If you're not finished but want me to look at your patch -- which is totally fine and a good idea if you have a question about something you're doing -- then use the "feedback" flag instead, in the same way as the review flag.

More info here, see Step 4: https://developer.mozilla.org/en-US/docs/Introduction
Flags: needinfo?(adw)
Thanks.

I would like to share a design suggestion. Instead of putting everything inside the parent DOM, I would suggest we show the log array contents in a popup box/window since the array size is pretty long. What would be the Firefox way to do it? 

I see the usage of Components.constructor in a few places but couldn't figure out how to create/use a similar popup-box/window. 

Please let me know your thoughts on the same.


Regards,
Shoubhik
Flags: needinfo?(adw)
Sure, we could do that.  We already do use a modal window for the "Show Update History" button in about:support, so we could do something like that again.  I'm not sure which would be a better UX, the thing I suggested or a modal popup.

To be clear, I wasn't suggesting showing the entire log at once.  I was saying that we could use a <pre> with a small max-height and overflow:auto, so that the user could scroll through the <pre> to see the entire log.

But yeah, you're definitely welcome to play with using a modal popup. :-)  It'll be more work though.

Take a look at how the Show Update History button works.  When you click it, it calls showUpdateHistory() on a prompter object.  Where is showUpdateHistory defined?  To find out, you can search mxr: http://mxr.mozilla.org/mozilla-central/search?string=showUpdateHistory  It's defined here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#4795

And this is the window that it opens: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/content/history.xul  You would need to create a new XUL file with a new <dialog>, like history.xul.

When you add new UI files, you also need to update (or add) these "jar.mn" files.  jar.mn files are manifests that list the UI files that should be packaged inside Firefox.  In the history.xul case, this is the jar.mn that lists it: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/jar.mn

Basically each line in that file maps the path of a source file (the right side) to a path in the URL through which you reference that file inside Firefox code (the left side).  The format of jar.mn is documented here:

https://dxr.mozilla.org/mozilla-central/source/build/docs/jar-manifests.rst
https://developer.mozilla.org/en-US/docs/JAR_Packaging

In the history.xul case, you can see that the file content/history.xul (relative to the jar.mn's directory) is mapped to the URL with a path of content/mozapps/update/history.xul.  Same for history.js.  Unfortunately, this whole mapping thing is unnecessarily complex, and that URL path actually ends up being something different for reasons I won't get into.  But you can see the final URL by looking up the URI constant that showUpdateHistory() passes to window.open(): http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#70

So once you add a new XUL (and JS) file for your popup, you need to also update or add the corresponding jar.mn.  Otherwise Firefox won't actually see your new file.

Let me know when you have questions.
Flags: needinfo?(adw)
Assignee: nobody → sbose78
Status: NEW → ASSIGNED
Drew, do we still need this? I might have some time to make it happen
Flags: needinfo?(adw)
Hi Fred, sure, feel free.
Assignee: sbose78 → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(adw)
took it
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
I use the pre area suggested in comment 4 to show logs
Attachment #8746959 - Flags: review?(adw) → review+
Comment on attachment 8746959 [details]
MozReview Request: Bug 522668 - support check and fix place database in about:support; r?adw

https://reviewboard.mozilla.org/r/49645/#review46563

This looks great, thanks.
thanks!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b1aed88e6306
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
This bug was about to implement PlacesDBUtils.checkAndFixDatabase(). I have seen the feature being with latest Beta 49.0b6 on Windows 7, 64 Bit!

This bug's bug's fix is verified on latest Beta
Build ID    20160822111414
User Agent  Mozilla/5.0 (WindowsNT 6.1; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
In https://support.mozilla.org/en-US/kb/use-troubleshooting-information-page-fix-firefox we currently have
"Use the [Verify Integrity] button to check the Places database and run maintenance tasks. This can fix problems with the bookmarks and history component of Firefox. "

I'd love to update it to be more specific about what types of problems this would help with, but I'm not sure. What types of problems might the Verify Integrity button fix?
Hi Chris, good question.  It's pretty technical, but a summary might be:

* tries to fix a corrupt database
* fixes data errors that may be present even in a non-corrupt database
* removes expired data
* frees up unused space, possibly making the database smaller
* logs some statistics about the database

Most of these things happen periodically without your having to do anything, but you can press the verify button to force them all to happen.

The code is here: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/PlacesDBUtils.jsm#116
Something like this?
- problems saving, accessing or managing bookmarks
- problems accessing browsing history
Hmm, I guess I would modify that to something like:

* fixes potential problems in the history and bookmarks database
* removes old, unnecessary information related to history and bookmarks

I think it's also important to stress that Firefox does these things automatically and periodically for you, so it's not like you have to press this button periodically.
Thank you. 
Yes, I think the fact that Firefox already does this on it's own is important info. I didn't have the impression that it did.
You need to log in before you can comment on or make changes to this bug.