Even if scripts are not fully loaded Lazy loader may incorrectly notify the caller about loading complete

VERIFIED FIXED

Status

Firefox OS
Gaia
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Jose Manuel Cantera, Unassigned)

Tracking

unspecified
x86
Mac OS X

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.1 fixed)

Details

(Whiteboard: QARegressExclude, [qa-])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
STR:

Launch LazyLoader.load for a set of script files. 
Launch LazyLoader.lad for the same set of files 

The second call will immediately return even if the scripts have not been loaded. 

The problem is in line https://github.com/mozilla-b2g/gaia/blob/master/shared/js/lazy_loader.js#L56 document.querySelector will return truthy even the file has not been loaded. 

As there is a variable _loading that maintains the scripts currently in loading, the querySelector is not really needed.
(Reporter)

Comment 1

5 years ago
Created attachment 715157 [details]
Pointer to GH PR
Attachment #715157 - Flags: review?(kgrandon)
Comment on attachment 715157 [details]
Pointer to GH PR

This patch is fine until we do it in the proper way. I would like to see a patch for this use case though, so please add one or assign to me to add. Perhaps we will need to play with setTimeouts to test this? E.g., delay the first script from being loaded but not the second, then ensure that the second one loads after the first?

Please either add a test - or assign to me and I will be more than happy to work on it if you are busy. Thanks!
Attachment #715157 - Flags: review?(kgrandon) → review+
(Reporter)

Comment 3

5 years ago
with tests passing and updated as requested

https://github.com/mozilla-b2g/gaia/commit/91c265fc9b1442c67c3abc3e39497c292412a09c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 4

5 years ago
We need this bug to land in v1.0.1 and v1-train as it blocks bug 852036 which is tef+
Blocks: 852036
blocking-b2g: --- → tef?

Updated

5 years ago
blocking-b2g: tef? → tef+
(Reporter)

Comment 5

5 years ago
uplifted to v1-train

https://github.com/mozilla-b2g/gaia/commit/1b48edd53ea25d2b46b2f5e763afc95ed766ce08

and to v1.0.1

https://github.com/mozilla-b2g/gaia/commit/5a174d81fd959e92f799998e0e6de97b8f583eed
(Reporter)

Updated

5 years ago
status-b2g18: --- → fixed
status-b2g18-v1.0.1: --- → fixed
Unable to run
Launch LazyLoader.load for a set of script files. 
Launch LazyLoader.lad for the same set of files

Tagging QARegressExclude
Whiteboard: QARegressExclude

Comment 7

5 years ago
Can we get the tools to test this?  Or can someone on TEF side verify this?
Flags: needinfo?(jmcf)
Whiteboard: QARegressExclude → QARegressExclude, [qa-]
(Reporter)

Updated

5 years ago
Status: RESOLVED → VERIFIED
(Reporter)

Updated

4 years ago
Flags: needinfo?(jmcf)
You need to log in before you can comment on or make changes to this bug.