Closed Bug 823547 Opened 8 years ago Closed 7 years ago

Store the snippets version along with the snippets

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 22

People

(Reporter: mak, Assigned: mak)

References

Details

(Whiteboard: [about-home])

Attachments

(1 file)

On snippets version bump, if we fail to cache the new snippets, we fallback to the cached ones, that are an old unsupported version.
We should store the version along with the snippets, so that we clear the cache if it's outdated.
Whiteboard: [about-home]
Depends on: 820834
While writing tests for this, I found the current browser_aboutHome.js test wrongly relies on bug 790934, or better, it relies on the fact we set the attributes twice!
I'm trying to fix it.
Attached patch patch v1.0Splinter Review
applies on top of the patch bug 820834.

When we bump the snippets version, this ensures we don't try to reuse a cached old version.

Unfortunately it's not 100% safe due to https://bugzilla.mozilla.org/show_bug.cgi?id=820834#c16 but that case should not be hit by final users (their browser should never ask for a future version).
Attachment #713640 - Flags: review?(gavin.sharp)
Comment on attachment 713640 [details] [diff] [review]
patch v1.0

Hoping Drew can help with review here.
Attachment #713640 - Flags: review?(gavin.sharp) → review?(adw)
Comment on attachment 713640 [details] [diff] [review]
patch v1.0

Review of attachment 713640 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/browser_aboutHome.js
@@ +171,5 @@
> +
> +    let snippetsElt = doc.getElementById("snippets");
> +    ok(snippetsElt, "Found snippets element");
> +    is(snippetsElt.getElementsByTagName("span").length, 1,
> +       "A default snippet is visible.");

"Visible" sounds like the wrong word, since this doesn't actually check the span's visibility.  "Present"?

@@ +194,5 @@
> +    let doc = gBrowser.selectedTab.linkedBrowser.contentDocument;
> +
> +    let snippetsElt = doc.getElementById("snippets");
> +    ok(snippetsElt, "Found snippets element");
> +    is(snippetsElt.innerHTML, "test", "Cached snippet is visible.");

Here too.
Attachment #713640 - Flags: review?(adw) → review+
https://hg.mozilla.org/mozilla-central/rev/1fea3c18dfd7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 848935
You need to log in before you can comment on or make changes to this bug.