Closed
Bug 908946
Opened 12 years ago
Closed 12 years ago
[Browser] Consider having DOM nodes imported from standalone files.
Categories
(Firefox OS Graveyard :: Gaia::Browser, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: kgrandon, Assigned: kgrandon)
References
Details
(Keywords: perf, Whiteboard: [c= p=0 s= u=] sprintready)
Attachments
(1 file)
222 bytes,
text/html
|
Details |
The app currently lazy loads the DOM via comments inside of HTML files. It should be possible to use our new import polyfill to have a cleaner dom.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Whiteboard: [c= p= s= u=] → [c= p=2 s= u=]
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #800400 -
Flags: review?(bfrancis)
Comment 2•12 years ago
|
||
Hi Kevin,
I'm not sure why you tried to sell this patch to me as "the benefit is that you get syntax highlighting for HTML!" in IRC, you could have just said "we'd like to start to support web components" which is much more interesting.
1) One of our biggest problems with the hardware we currently have is the speed of I/O. This change would seem to mean loading lots more files from non-volatile storage which I would expect to negatively effect performance. Have you made any efforts to actually measure the effect on performance?
2) I'd like to explore loading JS and CSS inside the corresponding HTML files as well.
3) Why are we rolling our own HTML Import/Web Components polyfill instead of using x-tags? Is it because we're concerned about performance?
4) Why are we initiating this via the lazy loader instead of using it directly? Is it to give us dynamic control over when different components are loaded?
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Ben Francis [:benfrancis] from comment #2)
> Hi Kevin,
>
> I'm not sure why you tried to sell this patch to me as "the benefit is that
> you get syntax highlighting for HTML!" in IRC, you could have just said
> "we'd like to start to support web components" which is much more
> interesting.
You're absolutely right. I'm a terrible salesman >_<
> 1) One of our biggest problems with the hardware we currently have is the
> speed of I/O. This change would seem to mean loading lots more files from
> non-volatile storage which I would expect to negatively effect performance.
> Have you made any efforts to actually measure the effect on performance?
We were aware of that, so we designed the import polyfill as something that took this into consideration. In production, we actually fall back to what browser/index.html looks like today. You can build a profile and inspect the contents in application.zip to see. That's why you can simply move HTML around with zero performance impact. The embed step is part of the build and is found here: https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-optimize.js#L261
> 2) I'd like to explore loading JS and CSS inside the corresponding HTML
> files as well.
This is definitely possible, and settings is already doing that. You can simply include css/script tags within each component and they will work. Though I think those should be follow-up bugs.
> 3) Why are we rolling our own HTML Import/Web Components polyfill instead of
> using x-tags? Is it because we're concerned about performance?
Correct - this is about performance. XHR is too slow on a device - see 1.
> 4) Why are we initiating this via the lazy loader instead of using it
> directly? Is it to give us dynamic control over when different components
> are loaded?
The lazy loader was already lazy loading DOM nodes - and browser was taking advantage of it. I mean lazy loading, by setting the comment text to innerHTML. Our import polyfill simply injects the comments at build time, instead of having them there in source control. To keep the performance gains of commented nodes, we need to have some javascript somewhere that populates them. LazyLoader just made sense as most apps already leveraged it. It should be certainly possible to integrate it without LazyLoader though should you have a use case.
Updated•12 years ago
|
Whiteboard: [c= p=2 s= u=] → [c= p=2 s= u=] sprintready
Comment 4•12 years ago
|
||
A Pivotal Tracker story has been created for this Bug: http://www.pivotaltracker.com/story/show/57483904
Comment 5•12 years ago
|
||
Comment on attachment 800400 [details]
Github Pull Request
Clearing the review flag because this has bitrotted (my bad) and I'd like to reduce churn on the browser app since it will soon be going away.
Attachment #800400 -
Flags: review?(bfrancis)
Assignee | ||
Comment 6•12 years ago
|
||
This is easy enough to rebase and fix, but if the browser app is going away soon-ish, I'm fine closing this out for now.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Whiteboard: [c= p=2 s= u=] sprintready → [c= p=0 s= u=] sprintready
You need to log in
before you can comment on or make changes to this bug.
Description
•