1222189 - Snooze unit dropdown menu items have no label, fix usage of getElementsByTagName due to Bug 492933

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Mark Schuster]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151103023037

Steps to reproduce:

Win 10 64bit, TB 45.0a1 (2015-11-05), Lighting 4.7a1

Adjusting the "Snooze (All) for" time of the Reminder dialog box does not populate the drop-down box with "Minutes, Hours, Days" for a custom time not listed.

Click on either "Snooze for" or "Snooze All for" buttons.
Adjust the time for something other than the defaults
Hover mouse over the drop-down box to select Minutes, Hours, Days



Actual results:

Drop-down is not populated with Minutes/Hours/Days

Comment 1

[Mark Schuster]

The Lightning home page tells us to report bugs to Bugzilla.

Comment 2

[Mark Schuster]

Attachment: Drop-down not populated

Comment 3

[Stefan Sitter [:ssitter]]

Confirmed using Lightning 4.6a2 in Thunderbird 44.0a2.

Comment 4

[Stefan Sitter [:ssitter]]

From a quick look it seems that in file calendar-alarm-widget.xml, method updateAccessibleName() the last for-loop is never entered and therefore the label of the xul:menuitems is never set: https://dxr.mozilla.org/comm-central/source/calendar/base/content/widgets/calendar-alarm-widget.xml#345

It looks like getElementsByTagName() doesn't return values. Searching for getElementsByTagName resulted in Bug 492933 that changed behavior in mozilla44. I therefore assume this is the cause for the regression.

Comment 5

[Stefan Sitter [:ssitter]]

Attachment: fix usage of getElementsByTagName

Updated according to <https://www.fxsitecompat.com/en-US/docs/2015/getelementsbytagname-now-matches-localname-instead-of-tagname/> and now it works for me.

Comment 6

[Mark Schuster]

(In reply to Stefan Sitter from comment #5)
> Created attachment 8684429 [details] [diff] [review]
> fix usage of getElementsByTagName
> 
> Updated according to
> <https://www.fxsitecompat.com/en-US/docs/2015/getelementsbytagname-now-
> matches-localname-instead-of-tagname/> and now it works for me.

Not sure what is meant by "...and now it works for me?"  I upgraded to 45.0a1 and still have the problem.

Comment 7

[Stefan Sitter [:ssitter]]

After applying the attached patch the drop-down menu entries have content/labels again. 
Mark, are you saying it doesn't work for you after applying the attached patch?

Comment 8

[Mark Schuster]

Stefan, I have to admit I do not know what the process is to add the attachment to TB to update it.  Can you point me in the right direction?

A sidebar: Do you know if there has been any activity on another of my bug reports: https://bugzilla.mozilla.org/show_bug.cgi?id=1222165

Thanks

Comment 9

[Stefan Sitter [:ssitter]]

Comment on attachment 8684429 [details] [diff] [review]
fix usage of getElementsByTagName

Matthew, maybe you can take over the review from Philipp? I just want this feature regression fixed before the next merge to beta branch.

Comment 10

[Matthew Mecca [:mmecca]]

Comment on attachment 8684429 [details] [diff] [review]
fix usage of getElementsByTagName

Looks good to me. r=mmecca

Comment 11

[Stefan Sitter [:ssitter]]

Pushed to https://hg.mozilla.org/comm-central/rev/65216fa64f21

Comment 12

[Stefan Sitter [:ssitter]]

Philipp, I'd like to ask you to take a look before the next branch uplift.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8684429 [details] [diff] [review]
fix usage of getElementsByTagName

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

Looks good to me, thanks for taking over the review.

Comment 14

[Stefan Sitter [:ssitter]]

Thanks.
Pushed to https://hg.mozilla.org/releases/comm-aurora/rev/595e301bf6b7

Comment 15

[Mark Schuster]

Thank You all for correcting this.

Comment 16

[Stefan Sitter [:ssitter]]

Thank you for reporting the problem. Please keep testing nightly builds and don't hesitate to report problems that you find.

Comment 17

[Stefan Sitter [:ssitter]]

Bug 492933 got backed out on all branches and therefore we have to revert the Lightning change on all branches down to 4.6 to fix it again.

Comment 19

[Stefan Sitter [:ssitter]]

Attachment: backout previous fix

Comment 20

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8714027 [details] [diff] [review]
backout previous fix

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

Thanks for keeping an eye on that bug! Can we just switch to querySelector or something that does not care about namespaces?

Anyway, as you already have the patch, I'm ok with taking it!

Comment 21

[Stefan Sitter [:ssitter]]

Pushed to comm-beta / Lightning 4.7: https://hg.mozilla.org/releases/comm-beta/rev/695604cffaa4

Pushed to comm-aurora / Lightning 4.8: https://hg.mozilla.org/releases/comm-aurora/rev/c608661fb128

Leave open for comm-central / Lightning 4.9.

Comment 22

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/0ad909848849967c10930861b8d49a6847d54c99
Bug 1222189 - Revert fix because getElementsByTagName change was backed out. r=Fallen

Comment 23

[Philip Chee]

Why not just use getElementsByTagNameNS ?