The default bug view has changed. See this FAQ.

CSS cut short in Style Editor

VERIFIED FIXED in Firefox 11

Status

()

Firefox
Developer Tools: Style Editor
P2
normal
VERIFIED FIXED
5 years ago
8 months ago

People

(Reporter: Stewart Souter, Assigned: cedricv)

Tracking

Trunk
Firefox 12
Points:
---

Firefox Tracking Flags

(firefox11+ verified, firefox12 verified)

Details

(Whiteboard: [qa!])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 584188 [details]
StyleEditor.jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:12.0a1) Gecko/20111223 Firefox/12.0a1
Build ID: 20111223032107

Steps to reproduce:

Just opened the Style Editor


Actual results:

the CSS displayed in the Style Editor is cut short or not being fully loaded. If you visit one of my sites, http://carbonize.co.uk/Lazarus/ and open style editor you will see that the code from style.css is not all there.


Expected results:

All the CSS from the file should be visible

Comment 1

5 years ago
I can reproduce. Cedric, can you please take a look at this?
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

5 years ago
OS: Windows 7 → All
Hardware: x86 → All
Version: 12 Branch → Trunk

Updated

5 years ago
Priority: -- → P2
I can also reproduce this bug in robcee's website: http://antennasoft.net/robcee/
(Assignee)

Updated

5 years ago
Assignee: nobody → cedricv
(Assignee)

Comment 3

5 years ago
Created attachment 586101 [details] [diff] [review]
Fix

I'm still unsure why the issue was triggered on some content and not others, hence the current lack of regression test [it seems to always pass when served through the mochitest web server].
Attachment #586101 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #586101 - Flags: review? → review?(rcampbell)

Comment 4

5 years ago
Comment on attachment 586101 [details] [diff] [review]
Fix

So you're treating an async IO stream synchronously.  available() doesn't really mean "number of bytes left before EOF" it means more like "number of bytes that have been read off the disk already in the IO thread".  So if you spin a loop waiting for number of read bytes to equal content-length, you're really just blocking the main thread long enough for the IO thread to read the whole file (which is bad).  This should be redone as a real async load.

Looking a bit further up, you're manually constructing a stream converter based on headers.  This isn't particularly maintainable.  Also, this seems to only work on things that were cached.

It seems like what you really want to do is construct an HTTP channel with the proper flags to prefer the cached resource, and let it figure out the correct encoding/caching semantics.

I can throw together a quick patch for this later today.
Attachment #586101 - Flags: review?(rcampbell) → review-
(Assignee)

Comment 5

5 years ago
Created attachment 586116 [details] [diff] [review]
Fix v2 - refactor using nsIChannel w/ LOAD_FROM_CACHE instead of manual nsIStreamConverter from cache

Wow. Thanks for that enlightenment, now makes much more sense :)

Tentative v2.
Attachment #586101 - Attachment is obsolete: true
Attachment #586116 - Flags: review?(dcamp)

Updated

5 years ago
Attachment #586116 - Flags: review?(dcamp) → review+
is this ready to land?
Status: NEW → ASSIGNED
Whiteboard: [land-in-fx-team?]
(Assignee)

Comment 7

5 years ago
Yes. And we'll likely need to later ask for beta approval as this is an important fix.
(Assignee)

Updated

5 years ago
Whiteboard: [land-in-fx-team?] → [land-in-fx-team]
(In reply to Cedric Vivier [cedricv] from comment #7)
> Yes. And we'll likely need to later ask for beta approval as this is an
> important fix.

should just need it in aurora as style editor isn't in beta yet, but yes! Most definitely.
tracking-firefox11: --- → ?

Comment 9

5 years ago
Please nominate for Aurora approval once this has had a couple of days of bake time on m-c.
tracking-firefox11: ? → +
(In reply to Alex Keybl [:akeybl] from comment #9)
> Please nominate for Aurora approval once this has had a couple of days of
> bake time on m-c.

that was the plan.
tracking-firefox11: + → ?

Updated

5 years ago
tracking-firefox11: ? → +
https://hg.mozilla.org/integration/fx-team/rev/fb9e08e72815
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/fb9e08e72815
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 12
Verified on Build identifier: Mozilla/5.0 (X11; Linux i686; rv:12.0a1) Gecko/20120125 Firefox/12.0a1

Comment 14

5 years ago
Comment on attachment 586116 [details] [diff] [review]
Fix v2 - refactor using nsIChannel w/ LOAD_FROM_CACHE instead of manual nsIStreamConverter from cache

[Approval Request Comment]
Regression caused by (bug #): New feature
User impact if declined: Nasty bug cutting CSS short.
Testing completed (on m-c, etc.): Been on m-c for almost a month.
Risk to taking this patch (and alternatives if risky): Honestly can't think of anything worse than the bug it fixes.  Code is simpler than the code it's replacing.
String changes made by this patch: None.
Attachment #586116 - Flags: approval-mozilla-beta?
Attachment #586116 - Flags: approval-mozilla-aurora?
Comment on attachment 586116 [details] [diff] [review]
Fix v2 - refactor using nsIChannel w/ LOAD_FROM_CACHE instead of manual nsIStreamConverter from cache

[Triage Comment]
Low risk and a poor user experience - approved for Aurora 12 and Beta 11.
Attachment #586116 - Flags: approval-mozilla-beta?
Attachment #586116 - Flags: approval-mozilla-beta+
Attachment #586116 - Flags: approval-mozilla-aurora?
Attachment #586116 - Flags: approval-mozilla-aurora+

Updated

5 years ago
Whiteboard: [land-in-aurora][land-in-beta]

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/57312e22d36c
status-firefox11: --- → fixed
Whiteboard: [land-in-aurora][land-in-beta] → [land-in-aurora]

Comment 17

5 years ago
This landed Jen16, before the fx12 aurora cutoff, so it's already on aurora.
status-firefox12: --- → unaffected
Whiteboard: [land-in-aurora]

Updated

5 years ago
status-firefox12: unaffected → ---
Whiteboard: [qa+]
Verified using the pages provided in the Description and Comment 2 - the CSS displayed in the Style Editor is complete.

Tested using Firefox 11 beta 4 on Windows 7, Ubuntu 11.10 and Mac OS X 10.6:

Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
status-firefox11: fixed → verified
status-firefox12: --- → fixed
Whiteboard: [qa+] → [qa+][qa!:11]
Verified as fixed on Firefox 12 beta 2 using the same steps from Comment 18 - the CSS displayed in the Style Editor is complete.

Verified on Windows 7, Ubuntu 11.10 and Mac OS X 10.6.
Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0

Setting resolution to VERIFIED FIXED.
Status: RESOLVED → VERIFIED
status-firefox12: fixed → verified
Whiteboard: [qa+][qa!:11] → [qa!]
Comment hidden (spam)

Updated

8 months ago
Flags: needinfo?(cedricv)
You need to log in before you can comment on or make changes to this bug.