The default bug view has changed. See this FAQ.

Create css for a "connecting" state in the url bar/tab progress lines

RESOLVED FIXED

Status

()

Firefox
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Margaret, Assigned: Margaret)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

7 years ago
Bug 544818 and bug 578028 add progress lines to tabs and the url bar. However, we need to create css for a "connecting" as shown in this mockup: https://bug578028.bugzilla.mozilla.org/attachment.cgi?id=470022.
(Assignee)

Updated

7 years ago
OS: Mac OS X → All
Hardware: x86 → All
Non negotiable for final release!
blocking2.0: --- → betaN+
Here's the mock-up: http://www.stephenhorlander.com/pages/progress-bar-line-mockup/progress-bar-line-states.html
the video in comment 2 doesn't seem to work for me, fwiw.
Bonus points if "Connecting" is different from "doing DNS resolution."
(Assignee)

Comment 5

7 years ago
Created attachment 478436 [details] [diff] [review]
WIP (pinstripe only)

There seems to be a little bit of a delay before it gets started, but I'm not sure why. 

It's also kind of difficult to test because you need to find a slow website. To get a better look I used DOM Inspector to set the value of the progressmeter to 0.
Attachment #478436 - Flags: feedback?(shorlander)
Attachment #478436 - Flags: feedback?(paul)
(Assignee)

Comment 6

7 years ago
Created attachment 478896 [details] [diff] [review]
patch v1

Based on the UX team meeting, I changed the patch to only implement this connecting state only for the url bar. I also added the css for winstripe/gnomestripe, but it needs testing.
Attachment #478436 - Attachment is obsolete: true
Attachment #478896 - Flags: feedback?(shorlander)
Attachment #478896 - Flags: feedback?(paul)
Attachment #478436 - Flags: feedback?(shorlander)
Attachment #478436 - Flags: feedback?(paul)
Note: if this gets reviewed before we freeze for b7, it'll become a b7 blocker. Hint, hint!
(Assignee)

Updated

7 years ago
Attachment #478896 - Attachment description: WIP v2 → patch v1
Attachment #478896 - Flags: review?(dao)
Comment on attachment 478896 [details] [diff] [review]
patch v1

> #urlbar-progress {
>-  -moz-binding: url("chrome://global/content/bindings/progressmeter.xml#progressmeter");
>+  -moz-binding: url("chrome://browser/content/urlbarBindings.xml#progressmeter-line");

Any reason not to call this urlbarBindings.xml#urlbar-progress?

>+  <binding id="progressmeter-line" extends="chrome://global/content/bindings/progressmeter.xml#progressmeter">
>+    <implementation>
>+      <constructor><![CDATA[
>+        this.addEventListener("transitionend", this.onTransitionEnd, true); 
>+      ]]></constructor>
>+      <destructor><![CDATA[
>+        this.removeEventListener("transitionend", this.onTransitionEnd, true);
>+      ]]></destructor>
>+      <method name="onTransitionEnd">
>+        <body><![CDATA[
>+          if (this.hasAttribute("slideBack") || this.value > 0)
>+            this.removeAttribute("slideBack");
>+          else
>+            this.setAttribute("slideBack", true);
>+        ]]></body>
>+      </method>
>+    </implementation>
>+  </binding>

Looks like an XBL <handler> should be used here.

Attribute names should be all-lowercase.
Attachment #478896 - Flags: review?(dao) → review-
(Assignee)

Comment 9

7 years ago
Created attachment 479700 [details] [diff] [review]
patch v2

Addressed review comments. I originally named the binding #progressbar-line when I thought we would also be using it for the tab progress lines, but since we're not I changed it to #urlbar-progress.
Attachment #478896 - Attachment is obsolete: true
Attachment #479700 - Flags: review?(dao)
Attachment #478896 - Flags: feedback?(shorlander)
Attachment #478896 - Flags: feedback?(paul)
Comment on attachment 479700 [details] [diff] [review]
patch v2

>+  <binding id="urlbar-progress" extends="chrome://global/content/bindings/progressmeter.xml#progressmeter">
>+    <implementation>
>+      <method name="onTransitionEnd">
>+        <body><![CDATA[
>+          if (this.hasAttribute("slideback") || this.value > 0)
>+            this.removeAttribute("slideback");
>+          else
>+            this.setAttribute("slideback", true);
>+        ]]></body>
>+      </method>
>+    </implementation>
>+    <handlers>
>+      <handler event="transitionend" action="this.onTransitionEnd();"/>
>+    </handlers>

Remove onTransitionEnd and put the code right inside the <handler>?

>+#urlbar-progress[value="0"] > .progress-remainder {
>+  background-image: -moz-linear-gradient(left, rgb(156,156,156), rgb(255,255,255), rgb(156,156,156)) !important;
>+  background-size: 10%;
>+  background-repeat: no-repeat;
>+  background-position: right;

Instead of rgb(156,156,156), shouldn't the edges of this image be transparent?

Is !important really needed here?
(Assignee)

Comment 11

7 years ago
Created attachment 479791 [details] [diff] [review]
patch v3

I'm slowly learning what you can do with xbl :)

I agree with you about the transparent edges. I tried removing the !important, but it turns out it is needed.
Attachment #479700 - Attachment is obsolete: true
Attachment #479791 - Flags: review?(dao)
Attachment #479700 - Flags: review?(dao)
(Assignee)

Comment 12

7 years ago
I just tested on Ubuntu and Windows 7, and it looks kind of bad because the background of the progressmeter isn't dark enough. However, it looks good when I also apply Stephen's patch from bug 597592, so that will take care of the polish.
Comment on attachment 479791 [details] [diff] [review]
patch v3

'transparent' translates to rgba(0,0,0,0), but for the gradient you probably want rgba(255,255,255,0) instead. You could also replace rgb(255,255,255) with 'white'.

r=me with that
Attachment #479791 - Flags: review?(dao) → review+
Go go go.
blocking2.0: betaN+ → beta7+
(Assignee)

Comment 15

7 years ago
Created attachment 480129 [details] [diff] [review]
final patch

http://hg.mozilla.org/mozilla-central/rev/c5c3d5c78727
Attachment #479791 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
This has an issue in Windows 7. The connect graphic animates behind the progress bar where much of it is obfuscated. Only a very faint glow is shown moving above the progress bar.
(Assignee)

Comment 17

7 years ago
(In reply to comment #16)
> This has an issue in Windows 7. The connect graphic animates behind the
> progress bar where much of it is obfuscated. Only a very faint glow is shown
> moving above the progress bar.

Thanks for testing. I think this will be fixed when the patch for bug 597592 lands, which I mentioned in comment 12.
This seems really CPU-intensive on my machine; when it's visible it takes up about 80% CPU.  I guess I should file a bug on that at some point, with a profile...
Blocks: 601851
(In reply to comment #11)
> I tried removing the !important,
> but it turns out it is needed.
I guess this is only needed because of the place of these rules on the stylesheet. These rules should be placed *after*:

1134 .tab-progress > .progress-remainder,
1135 #urlbar-progress > .progress-remainder {
1136   border-top: 1px solid rgba(100,100,100,.1);
1137   border-bottom: 1px solid rgba(0,0,0,.2);
1138   background-image: -moz-linear-gradient(right, ThreeDShadow 80%, ThreeDLightShadow);
1139 }
1140 
1141 .tab-progress > .progress-remainder:-moz-locale-dir(rtl),
1142 #urlbar-progress > .progress-remainder:-moz-locale-dir(rtl) {
1143   background-image: -moz-linear-gradient(left, ThreeDShadow 80%, ThreeDLightShadow);
1144 }

I'm also noticing some high CPU usage with this.

Ah... I was only able to see this effect adding a background-color to the set of rules. Setting it to highlight works ok for me. I'm on Windows XP.

Updated

7 years ago
Depends on: 602126

Comment 20

7 years ago
Bug 602126 - Excessive CPU usage due to progress lines, was filed for the high CPU usage

Comment 21

7 years ago
Comment on attachment 480129 [details] [diff] [review]
final patch

>diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml
>--- a/browser/base/content/urlbarBindings.xml
>+++ b/browser/base/content/urlbarBindings.xml

>+  
>+  <binding id="urlbar-progress" extends="chrome://global/content/bindings/progressmeter.xml#progressmeter">
>+    <handlers>
>+      <handler event="transitionend"><![CDATA[
>+        if (this.hasAttribute("slideback") || this.value > 0)
>+          this.removeAttribute("slideback");

That looks odd - it tries to remove attribute even when it hasn't got one.

>+        else
>+          this.setAttribute("slideback", true);
>+      ]]></handler>
>+    </handlers>
>+  </binding>
>+  
> </bindings>

Comment 22

7 years ago
(In reply to comment #21)
> 
> That looks odd - it tries to remove attribute even when it hasn't got one.
> 

By which I mean that I would expect something more like:

if (this.hasAttribute("slideback"))
  this.removeAttribute("slideback");
else if (this.value == 0)
  this.setAttribute("slideback", true);
(Assignee)

Comment 23

7 years ago
(In reply to comment #22)
> (In reply to comment #21)
> > 
> > That looks odd - it tries to remove attribute even when it hasn't got one.
> > 
> 
> By which I mean that I would expect something more like:
> 
> if (this.hasAttribute("slideback"))
>   this.removeAttribute("slideback");
> else if (this.value == 0)
>   this.setAttribute("slideback", true);

You're right. I'm not sure how I came to what we have now. I can fix it in the patch for bug 601851, since that's resolving other problems with this patch. Thanks for pointing that out!
You need to log in before you can comment on or make changes to this bug.