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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 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

9 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

9 years ago
OS: Mac OS X → All
Hardware: x86 → All
Non negotiable for final release!
blocking2.0: --- → betaN+
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

9 years ago
Posted patch WIP (pinstripe only) (obsolete) — Splinter Review
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

9 years ago
Posted patch patch v1 (obsolete) — Splinter Review
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

9 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

9 years ago
Posted patch patch v2 (obsolete) — Splinter Review
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

9 years ago
Posted patch patch v3 (obsolete) — Splinter Review
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

9 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

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 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

9 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...
(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

9 years ago
Depends on: 602126

Comment 20

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

Comment 21

9 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

9 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

9 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.