Closed
Bug 701635
Opened 14 years ago
Closed 14 years ago
Style Editor should have a page loading placeholder
Categories
(DevTools :: General, enhancement)
DevTools
General
Tracking
(firefox11- verified)
RESOLVED
FIXED
Firefox 12
People
(Reporter: cedricv, Assigned: cedricv)
Details
(Whiteboard: [styleeditor][qa+])
Attachments
(1 file, 1 obsolete file)
12.45 KB,
patch
|
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Quickly opening the Style Editor while the page is still loading shows the "This page has no style sheet" placeholder.
There should rather be a "The page is is still loading" placeholder as soon it is known the page contains style sheets.
Assignee | ||
Comment 1•14 years ago
|
||
Implemented in add-on v0.5.4 :
https://github.com/neonux/StyleEditor/commit/cb91c59cccecd2092dbf1a0272ba36e35d6e2917
Browser patch after landing.
Assignee | ||
Comment 2•14 years ago
|
||
It just shows a throbber in the center of the (empty) style sheet list [if and while the page is still loading].
I thought that there's no need to be more verbose.
Any objection/opinion Shorlander?
We'll probably need a DevTools-specific throbber too at some point (to match with the dark design).
Assignee | ||
Comment 3•14 years ago
|
||
Assignee | ||
Comment 4•14 years ago
|
||
This might be worth an aurora approval request (?).
The currently landed behavior can be quite confusing for users on slow connections.
Assignee | ||
Updated•14 years ago
|
Attachment #588045 -
Flags: review? → review?(rcampbell)
Comment 5•14 years ago
|
||
Comment on attachment 588045 [details] [diff] [review]
Display a throbber while the page is still loading instead of "this page has no stylesheet"
+ // (re)enable UI
+ let matches = this._root.querySelectorAll("toolbarbutton,input,select");
+ for (let i = 0; i < matches.length; ++i) {
+ matches[i].removeAttribute("disabled");
+ }
hm! OK.
...
CSS looks ok to me. Yay, throbbers!
Attachment #588045 -
Flags: review?(rcampbell) → review+
Comment 6•14 years ago
|
||
feel free to send this through try. If you think it'll pass without one, add [land-in-fx-team].
After landing, we can request aurora approval.
Comment 7•14 years ago
|
||
Comment on attachment 588045 [details] [diff] [review]
Display a throbber while the page is still loading instead of "this page has no stylesheet"
>--- a/browser/themes/pinstripe/devtools/splitview.css
>+++ b/browser/themes/pinstripe/devtools/splitview.css
>+.loading .splitview-nav-container {
>+ background-image: url(chrome://browser/skin/tabbrowser/loading.png);
Don't reference images in tabbrowser/ here. Either copy it or use chrome://global/skin/icons/loading_16.png.
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #588045 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Whiteboard: [styleeditor] → [styleeditor][land-in-fx-team]
Comment 9•14 years ago
|
||
Whiteboard: [styleeditor][land-in-fx-team] → [styleeditor][fixed-in-fx-team]
Comment 10•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [styleeditor][fixed-in-fx-team] → [styleeditor]
Target Milestone: --- → Firefox 12
Updated•14 years ago
|
tracking-firefox11:
--- → ?
Comment 11•14 years ago
|
||
Comment on attachment 589093 [details] [diff] [review]
patch v2 using global throbber instead of tabbrowser's
[Approval Request Comment]
Regression caused by (bug #): New Feature. Not a regression.
User impact if declined: Users won't know if their style sheets are loading or not.
Testing completed (on m-c, etc.): on m-c.
Risk to taking this patch (and alternatives if risky): Low. Mostly styling. Alternative is users don't get this functionality until fx 12.
Hey lookit the little auto-created bugzilla form! Neat.
Attachment #589093 -
Flags: approval-mozilla-aurora?
Comment 12•14 years ago
|
||
Comment on attachment 589093 [details] [diff] [review]
patch v2 using global throbber instead of tabbrowser's
[Triage Comment]
Low risk, prevents user confusion with new feature. Approved for Aurora 11.
Attachment #589093 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•14 years ago
|
||
status-firefox11:
--- → fixed
Comment 14•14 years ago
|
||
Will not apply because of missing dependent patch. Not sure what to do about that.
adding placeholder to series file
applying placeholder
patching file browser/devtools/styleeditor/StyleEditorChrome.jsm
Hunk #1 FAILED at 150
Hunk #2 FAILED at 293
2 out of 2 hunks FAILED -- saving rejects to file browser/devtools/styleeditor/StyleEditorChrome.jsm.rej
patching file browser/devtools/styleeditor/splitview.css
Hunk #1 FAILED at 40
1 out of 1 hunks FAILED -- saving rejects to file browser/devtools/styleeditor/splitview.css.rej
patching file browser/devtools/styleeditor/styleeditor.xul
Hunk #1 FAILED at 48
1 out of 1 hunks FAILED -- saving rejects to file browser/devtools/styleeditor/styleeditor.xul.rej
patching file browser/devtools/styleeditor/test/browser_styleeditor_loading.js
Hunk #1 FAILED at 12
Hunk #2 FAILED at 29
2 out of 2 hunks FAILED -- saving rejects to file browser/devtools/styleeditor/test/browser_styleeditor_loading.js.rej
patching file browser/themes/gnomestripe/devtools/splitview.css
Hunk #1 FAILED at 35
1 out of 1 hunks FAILED -- saving rejects to file browser/themes/gnomestripe/devtools/splitview.css.rej
patching file browser/themes/gnomestripe/devtools/styleeditor.css
Hunk #1 FAILED at 30
1 out of 1 hunks FAILED -- saving rejects to file browser/themes/gnomestripe/devtools/styleeditor.css.rej
patching file browser/themes/pinstripe/devtools/splitview.css
Hunk #1 FAILED at 35
1 out of 1 hunks FAILED -- saving rejects to file browser/themes/pinstripe/devtools/splitview.css.rej
patching file browser/themes/pinstripe/devtools/styleeditor.css
Hunk #1 FAILED at 30
1 out of 1 hunks FAILED -- saving rejects to file browser/themes/pinstripe/devtools/styleeditor.css.rej
patching file browser/themes/winstripe/devtools/splitview.css
Hunk #1 FAILED at 35
1 out of 1 hunks FAILED -- saving rejects to file browser/themes/winstripe/devtools/splitview.css.rej
patching file browser/themes/winstripe/devtools/styleeditor.css
Hunk #1 FAILED at 30
1 out of 1 hunks FAILED -- saving rejects to file browser/themes/winstripe/devtools/styleeditor.css.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh placeholder
Comment 15•14 years ago
|
||
disregard that. already in. because awesom.
Updated•13 years ago
|
Comment 16•13 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Still able to catch the issue if opening very quick the style editor (SHIFT+F7) after pressing ENTER in the location bar: www.cnn.com
Assignee | ||
Comment 17•13 years ago
|
||
(In reply to Paul Silaghi [QA] from comment #16)
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
> Still able to catch the issue if opening very quick the style editor
> (SHIFT+F7) after pressing ENTER in the location bar: www.cnn.com
Hmm, can you please open a new bug for this with detailed steps to reproduce?
Also, are you able to repro on every platform?
Comment 18•13 years ago
|
||
New bug 725222 filed.
Marking this as verified fixed on Firefox 11, since the editor works fine in normal conditions:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; 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
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•