Find an alternative to XUL:label for the stylesheet links

VERIFIED FIXED in Firefox 52

Status

defect
P1
normal
VERIFIED FIXED
3 years ago
Last year

People

(Reporter: jdescottes, Assigned: jdescottes)

Tracking

Trunk
Firefox 52
Dependency tree / graph

Firefox Tracking Flags

(firefox52 verified)

Details

(Whiteboard: [devtools-html])

Attachments

(3 attachments, 1 obsolete attachment)

This bug is blocking Bug 1291049.

The rule view is still relying on a XUL label for the stylesheet links. They are using the crop=center attribute in order to have a centered dynamic ellipsis.

In order to load the inspector in a content tab, we must remove all remaining XUL elements, so we should investigate an HTML based alternative.
Assignee

Comment 2

3 years ago
I found the following threads/docs about the possibility of adding middle cropping features to the text-overflow CSS property:
- https://www.w3.org/wiki/Text-overflow_middle_cropping
- Bug 740910

In the meantime we should try to find an alternative.
Comment hidden (mozreview-request)
Assignee

Comment 4

3 years ago
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

This patch is a CSS only workaround using the usual RTL workaround to crop the link at the beginning.

I think we can consider the "end" part of a stylesheet link to be the most meaningful. Still not as good as middle cropping maybe though.

If this is not ok, we can either look at JS alternatives or maybe have different implementations when in content and in chrome.
Attachment #8801334 - Flags: review?(pbrosset) → feedback?(pbrosset)
Comment hidden (mozreview-request)

Comment 6

3 years ago
mozreview-review
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

https://reviewboard.mozilla.org/r/86114/#review84728

::: devtools/client/themes/rules.css:146
(Diff revision 1)
> +  /* Workaround to crop the source link at the beginning. */
> +  overflow: hidden;
> +  text-overflow: ellipsis;
> +  direction: rtl;

I would direction: rtl just below the comment because it's more relevant to it.
Flags: qe-verify?
Whiteboard: [devtools-html]
Comment hidden (mozreview-request)

Comment 8

3 years ago
mozreview-review
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

https://reviewboard.mozilla.org/r/86114/#review84916

::: devtools/client/themes/rules.css:146
(Diff revision 1)
> +  /* Workaround to crop the source link at the beginning. */
> +  overflow: hidden;
> +  text-overflow: ellipsis;
> +  direction: rtl;

The RTL direction can break the formatting of text inside the .ruleview-rule-source element. See bug 1290056 to see what can happen and how to fix it.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Iteration: --- → 52.2 - Oct 17
Priority: P2 → P1
Assignee

Updated

3 years ago
Attachment #8801334 - Flags: review?(pbrosset) → feedback?(pbrosset)
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

The changes look good to me. How does this behave in an RTL browser though? Does it still crop the source at the beginning?
I also agree that keeping the end of the URL visible is the most useful we can do under the circumstances.
Attachment #8801334 - Flags: feedback?(pbrosset) → feedback+
(In reply to Patrick Brosset <:pbro> from comment #11)
> Comment on attachment 8801334 [details]
> Bug 1310310 - ruleview stylesheet link using span instead of label;
> 
> The changes look good to me. How does this behave in an RTL browser though?
> Does it still crop the source at the beginning?

Works fine in RTL direction as well. The direction: rtl; on `.ruleview-rule-source` becomes redundant with the inherited value, but the behavior remains the same. Screenshot attached. 

> I also agree that keeping the end of the URL visible is the most useful we
> can do under the circumstances.
Comment hidden (mozreview-request)
Assignee

Updated

3 years ago
Attachment #8801337 - Attachment is obsolete: true
Comment hidden (mozreview-request)

Comment 15

3 years ago
mozreview-review
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

https://reviewboard.mozilla.org/r/86114/#review85520

Looks and works great, thanks! It's a shame that left-side cropping needs to be this complicated in CSS and that crop:center is not possible at all.
Attachment #8801334 - Flags: review?(jsnajdr) → review+
Assignee

Comment 16

3 years ago
mozreview-review-reply
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

https://reviewboard.mozilla.org/r/86114/#review84728

Thanks for having a look Tim.

> I would direction: rtl just below the comment because it's more relevant to it.

Fixed, good point!
Assignee

Comment 17

3 years ago
mozreview-review-reply
Comment on attachment 8801334 [details]
Bug 1310310 - ruleview stylesheet link using span instead of label;

https://reviewboard.mozilla.org/r/86114/#review84916

Thanks for the review Jarda! Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=45e96df694571fd5ac155b623a48a99185cfe9b5
Try looks green after getting a lot of unrelated "1266624 Intermittent-infra [test-linux.sh:error] failed to download mozharness zip". Landing.

Comment 19

3 years ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b5f08127448
ruleview stylesheet link using span instead of label;r=jsnajdr

Comment 20

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8b5f08127448
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Flags: qe-verify? → qe-verify+
QA Contact: cristian.comorasu
Some pointers to verify this bug!

This bug modified the way stylesheet links are rendered in the inspector rule-view tab (see attachment 8801313 [details]). 

We should perform a non-regression on the display of those links. Try to visit websites with stylesheet links of different length, play with the available width for the rule view in order to force overflows, ellipses etc...

Please note that there is one known difference/regression compared to the previous behavior: the ellipsis used to appear in the middle of the link. It now appears on the left-side. This is a technical limitation and we won't be able to change this.
Thank you Julian Descottes for the guideline!

I reproduced this bug using Fx 52.0a1, build ID: 20161014060324, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 52.0a1, build ID: 20161113030203, on Windows 10 x64, Mac OS X 10.12 and Ubuntu 14.04 LTS.

Cheers!
Status: RESOLVED → VERIFIED
Flags: qe-verify+

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.