1008825 - Synchronous XMLHttpRequest to load options.xul

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Add-on Manager, defect)

Description

[:Margaret Leibovic]

This landed as part of bug 704415, and I see no comments in there justifying this, so I think it's just a mistake that this isn't asynchronous. Fixing this will involve flipping the async parameter, as well as updating the logic to be asynchronous.

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/aboutAddons.js#327

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Spoke with vik1231 on IRC.

Comment 2

[:Margaret Leibovic]

Thanks, mcomella!

I'm traveling this week, so feel free to post questions in the bug if I'm not responsive on IRC.

Comment 3

[vikneshwar]

Attachment: async.patch

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 8449727 [details] [diff] [review]
async.patch

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

Nice! Just make sure you get the indentation fixed up right (two space indent in here) :)

::: mobile/android/chrome/content/aboutAddons.js
@@ +336,1 @@
>          // Only allow <setting> for now

NIT: Line this up with the code below.

@@ +343,1 @@
>                setting.setAttribute("desc", desc);

Don't forget to fix this indent. Add curly braces as well. We decided to change styles and use them from now on.

@@ +356,4 @@
>            // No options, so hide the header and reset the list item
>            detailItem.setAttribute("optionsURL", "");
>            aListItem.setAttribute("optionsURL", "");
> +          }

And fix the indent here as well.

Comment 5

[vikneshwar]

Attachment: async-req

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/ec6f635ab57b

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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