1102171 - [Usage][Refactoring] ViewManager.loadPanel should cleanup panel's DOM once scripts/links are activated

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[Oleg Zasypkin [:azasypkin]]

To activate panel's nested scripts and links in ViewManager.loadPanel we create new nodes with the same src/href links and append it to the document head. In this bug we should try to _move_ nested resource nodes directly to the head instead or at least remove it from panel's DOM once they're activated.

See bug 1100915 comment 4 for details.

Comment 1

[Oleg Zasypkin [:azasypkin]]

> try to _move_ nested resource nodes directly to the head instead

As per :bz it won't work because script node cloned from one that was inserted via innerHTML is still "marked" as not-to-be-loaded-or-executed. So we'll need to create new script nodes anyway.

Comment 2

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Patch is ready, waiting for bug 1100915 to be landed before asking for review.

Comment 3

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8526790 [details] [review]
GitHub pull request URL

Hey Salvador,

Could you please share your feedback on this patch? I have some questions that I left on Github.

Thanks!

Comment 4

[Salvador de la Puente González [:salva]]

Comment on attachment 8526790 [details] [review]
GitHub pull request URL

My comments on GitHub, nothing remarkable. Good work!

Comment 5

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8526790 [details] [review]
GitHub pull request URL

(In reply to Salvador de la Puente González [:salva] from comment #4)
> Comment on attachment 8526790 [details] [review]
> GitHub pull request URL
> 
> My comments on GitHub, nothing remarkable. Good work!

Thanks for feedback and replies! Per feedback, no code changes requested yet, so asking for review :)

Thanks!

Comment 6

[Salvador de la Puente González [:salva]]

Comment on attachment 8526790 [details] [review]
GitHub pull request URL

No problems with this implementation but before landing, can you confirm if `cloneNode` could work instead of regenerating the node from scratch? Thank you Oleg.

Comment 7

[Oleg Zasypkin [:azasypkin]]

(In reply to Salvador de la Puente González [:salva] from comment #6)
> Comment on attachment 8526790 [details] [review]
> GitHub pull request URL
> 
> No problems with this implementation but before landing, can you confirm if
> `cloneNode` could work instead of regenerating the node from scratch? Thank
> you Oleg.

Nope, unfortunately cloneNode copies these script "hidden state" flags as well.

Thanks!

Comment 8

[Oleg Zasypkin [:azasypkin]]

Treeherder is green, landed.

Master: https://github.com/mozilla-b2g/gaia/commit/ee6443a912f75267b8fb51fd67909882ca2b5f4b

Thanks!