1583844 - "Last updated" field on Mozilla themes shows invalid date

Status: RESOLVED FIXED

(WebExtensions :: Themes, defect, P3)

Description

[Marcia Knous [:marcia]]

Attachment: Screen Shot 2019-09-25 at 10.04.22 AM.png

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

Comment 1

[Mark Striemer [: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?

Comment 2

[Tomislav Jovanovic :zombie]

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.

Comment 3

[Mark Striemer [:mstriemer]]

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

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

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

Comment 6

[Ajitesh13]

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

Comment 7

[Mark Striemer [:mstriemer]]

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.

Comment 8

[Ajitesh13]

Attachment: Bug 1583844 - "Last updated" field on Mozilla themes shows invalid date r=zombie

Comment 9

[Ajitesh13]

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

Comment 10

[Tomislav Jovanovic :zombie]

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

Comment 11

[Caitlin Neiman [:caitmuenster]]

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.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 13

[aarushivij]

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

Comment 14

[Ajitesh13]

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

Comment 15

[aarushivij]

No problem :)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d4ad032434f8
"Last updated" field on Mozilla themes shows invalid date r=zombie

Comment 17

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d4ad032434f8