Closed Bug 1583844 Opened 1 year ago Closed 5 months ago

"Last updated" field on Mozilla themes shows invalid date

Categories

(WebExtensions :: Themes, defect, P3)

Desktop
macOS
defect

Tracking

(firefox-esr68 unaffected, firefox75 wontfix, firefox76 wontfix, firefox77 fixed)

RESOLVED FIXED
mozilla77
Tracking Status
firefox-esr68 --- unaffected
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- fixed

People

(Reporter: marcia, Assigned: aji.yash13, Mentored)

References

(Regression)

Details

(Keywords: good-first-bug, regression)

Attachments

(2 files)

Seen while running nightly on macOS. See attached screenshot. "Invalid date" shows for the dark theme and for the default theme.

Flags: needinfo?(mstriemer)

Can confirm on local build Nightly, but on my installed Nightly, Beta and release I don't see this behaviour. I put a breakpoint where we set this and addon.updateDate is Invalid Date, so this seems expected, but obviously not useful.

This is a regression from bug 1543090, which stopped reading the modified time for built-in extensions. We could fix it in the add-on manager by hiding the date if it's invalid, but maybe built-in add-ons should have a valid date set for updateDate?

Zombie, looks like you reviewed the patch from bug 1543090, do you think there should be a valid updateDate on these add-ons? There's already a truthy check for the property, so maybe it should be a valid date or null?

Flags: needinfo?(mstriemer) → needinfo?(tomica)

Yeah, it looks this not a problem in existing profiles with new builds, since the DB already has the data, but i see the invalid date in a new profile.

I put a breakpoint where we set this and addon.updateDate is Invalid Date, so this seems expected, but obviously not useful.

Where does the Invalid Date string come from? It definitely sounds like that property should be null instead.

More generally, I don't expect us to need the actual date for any other use, since I believe the only way to update builtin addons is by installing a new version via the user profile.

And hiding a null date in addons details seems appropriate overall. I would expect that field to show the date of install or the latest update, which doesn't make sense for builtin addons that have not been updated.

Flags: needinfo?(tomica)

Looks like the getter is always casting the value to a Date [1]. Returning null there instead of creating a Date from undefined will hide it from about:addons. I'm not sure what test should be updated for that though.

[1] https://searchfox.org/mozilla-central/rev/23f836a71cfe961373c8bd0d0219ec60a64b3c8f/toolkit/mozapps/extensions/internal/XPIDatabase.jsm#1301

Assignee: nobody → tomica
Priority: -- → P2
Duplicate of this bug: 1594019
Priority: P2 → P3

Marked as good first bug and moved :zombie from assignee to mentor as agreed in the bug triaging meeting.

Assignee: tomica → nobody
Mentor: tomica
Keywords: good-first-bug

Hey, is it still open ? Can i take this bug ?

This is still a valid bug. There's a suggestion in comment 3 of what to do to fix it. Once you've got a patch you can upload it to phabricator and send the review to :zombie.

Assignee: nobody → aji.yash13
Status: NEW → ASSIGNED

I have submitted a patch on phabricator, please let me know, if i am wrong somewhere and any further changes needed.

Flags: needinfo?(tomica)

Thanks for the patch and sorry for the delay. Responded on phabricator.

Flags: needinfo?(tomica)

Hi Ajitesh, thanks for letting other folks work on this bug. :) I'm formally unassigning you from this bug and opening it up for other contributors.

Assignee: aji.yash13 → nobody
Status: ASSIGNED → NEW
Assignee: nobody → aji.yash13
Status: NEW → ASSIGNED

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

Hello, Can I work on this bug?
Thanks :)
Aarushi

Flags: needinfo?(tomica)
Flags: needinfo?(cneiman)

Hello Aarushi, I have already submitted a patch for this bug and was waiting for response to my query in phabricator. I am working on this for a while, so can you please look over another bug to work on ?

Thanks

No problem :)

Flags: needinfo?(tomica)
Flags: needinfo?(cneiman)
Flags: needinfo?(tomica)
Flags: needinfo?(tomica)
Flags: needinfo?(tomica)
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d4ad032434f8
"Last updated" field on Mozilla themes shows invalid date r=zombie
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Regressions: 1634756
Flags: needinfo?(tomica)
You need to log in before you can comment on or make changes to this bug.