Remove the resizer binding

RESOLVED FIXED in Firefox 61

Status

()

P5
normal
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: bgrins, Assigned: timdream)

Tracking

(Blocks: 3 bugs)

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(10 attachments)

59 bytes, text/x-review-board-request
enndeakin
: review+
Details
59 bytes, text/x-review-board-request
dao
: review+
Details
59 bytes, text/x-review-board-request
enndeakin
: review+
Details
59 bytes, text/x-review-board-request
enndeakin
: review+
Details
59 bytes, text/x-review-board-request
enndeakin
: review+
Details
59 bytes, text/x-review-board-request
enndeakin
: review+
Details
59 bytes, text/x-review-board-request
enndeakin
: review+
Details
59 bytes, text/x-review-board-request
enndeakin
: review+
Details
1.24 KB, image/svg+xml
Details
59 bytes, text/x-review-board-request
heycam
: review+
Details
(Reporter)

Description

11 months ago
This is an in-content binding created for resizing elements. It gets created here: https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/layout/generic/nsGfxScrollFrame.cpp#4696.

- We need to figure out if there's a way to move the writing mode logic out of the XBL constructor and into the place the anon content is created instead: https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/toolkit/content/widgets/resizer.xml#15.
- We need to figure out some way to load resizer.css in-content (either always import in minimal-xul.css, load as a separate stylesheet lazily when the NAC is created, or move the styles to be inline. Hopefully we can switch resizer.png and resizer@2x.png to svg which would simplify them a bit.
Thank you for the explanation, it's very straightforward to work with than I think it would. I have a WIP already.
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Comment 4

11 months ago
mozreview-review
Comment on attachment 8963903 [details]
Bug 1450017 - Part VI, Remove the resizer binding

https://reviewboard.mozilla.org/r/232758/#review238204

::: toolkit/content/minimal-xul.css:77
(Diff revision 1)
>  }
>  
>  /********** resizer **********/
>  
> +/* XXX Doesn't work, need investigate */
> +@import url("chrome://global/skin/resizer.css");

The import has to happen at the top of the file above all other rules
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Comment 7

11 months ago
I guess we'll also need to figure out how to deal with non-anon resizers like those used in reftests: https://searchfox.org/mozilla-central/search?q=%3Cresizer&case=true&path=. Maybe we need to set the attribute somehow in the xul element creation instead of in the frame.
(In reply to Brian Grinstead [:bgrins] from comment #7)
> I guess we'll also need to figure out how to deal with non-anon resizers
> like those used in reftests:
> https://searchfox.org/mozilla-central/search?q=%3Cresizer&case=true&path=.
> Maybe we need to set the attribute somehow in the xul element creation
> instead of in the frame.

They might not break - will test locally for sure - since they don't test writing modes (and <resizer rtl="true">).

Honestly if they do we should just delete them; we should not support a feature that Firefox no longer uses.
(Reporter)

Comment 9

11 months ago
:shorlander, can you help us get svg version(s) of the resizer icons used for textareas? As you mentioned we may not need to keep the three OS variants, but if we do end up wanting to it'd be fine as well.

* https://searchfox.org/mozilla-central/source/toolkit/themes/linux/global/icons/resizer.png
* https://searchfox.org/mozilla-central/source/toolkit/themes/windows/global/icons/resizer.png
* https://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/icons/resizer.png
* https://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/icons/resizer@2x.png
Flags: needinfo?(shorlander)
(Reporter)

Comment 10

11 months ago
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #8)
> (In reply to Brian Grinstead [:bgrins] from comment #7)
> > I guess we'll also need to figure out how to deal with non-anon resizers
> > like those used in reftests:
> > https://searchfox.org/mozilla-central/search?q=%3Cresizer&case=true&path=.
> > Maybe we need to set the attribute somehow in the xul element creation
> > instead of in the frame.
> 
> They might not break - will test locally for sure - since they don't test
> writing modes (and <resizer rtl="true">).
> 
> Honestly if they do we should just delete them; we should not support a
> feature that Firefox no longer uses.

When I first saw the list of tests I was thinking the reftests were used to confirm that the anon version gets drawn onto a textarea, etc but looking through it again that doesn't seem to be the case. We'll still need code that tests the widget to make sure that resizing works, but now I'm wondering if we only ever test the resizer in the non-anonymous case (which wouldn't be great since AFAICT it's only ever used in the content document as NAC).
(In reply to Brian Grinstead [:bgrins] from comment #10)
> When I first saw the list of tests I was thinking the reftests were used to
> confirm that the anon version gets drawn onto a textarea, etc but looking
> through it again that doesn't seem to be the case. We'll still need code
> that tests the widget to make sure that resizing works, but now I'm
> wondering if we only ever test the resizer in the non-anonymous case (which
> wouldn't be great since AFAICT it's only ever used in the content document
> as NAC).

There are reftests added in bug 1094434, comparing textarea of different writing modes with bidi. They should be kept.

Let me figure out if non-anonymous cases break and we will know if we need to do anything.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
So we would need to keep all the explicit non-anonymous case of <resizer> in reftest because they serve as references to the correct styling/positioning of anonymous <resizer> generated.

One reftest caught the failure caused by extra CSS rules copied from osx/resizer.css.

Specifically,

min-width/height was added because of
https://bugzilla.mozilla.org/show_bug.cgi?id=636564#c71

The comment here suggests we could just remove it, so I am going to try that.

hiding resizer[type="window] was added in

https://bugzilla.mozilla.org/show_bug.cgi?id=556645

For bug 556645 I am struggling to find a modern example since all preferences dialogs are in-content already. Do we still use resizable dialog anywhere in Firefox? If not, it's probably fine to regress bug 556645.

$ diff toolkit/themes/osx/global/resizer.css toolkit/themes/windows/global/resizer.css
12d11
<   min-width: 15px;
14d12
<   min-height: 15px;
17,26d14
< @media (min-resolution: 2dppx) {
<   resizer {
<     background-image: url("chrome://global/skin/icons/resizer@2x.png");
<     background-size: 100% 100%;
<   }
< }
<
< resizer[type="window"] {
<   display: none;
< }
32,39d19
< @media (min-resolution: 2dppx) {
<   resizer[rtl="true"],
<   resizer[dir="bottomend"]:-moz-locale-dir(rtl) {
<     background-image: url("chrome://global/skin/icons/resizer-rtl@2x.png");
<     background-size: 100% 100%;
<   }
< }
<
The other thing worth pointing out is that bug 636564 comment 71 at the time assumes non-mac scrollbars are all 15px wide. This is false at least on present-day Ubuntu. I don't know if we want to design a new SVG resizer handle that would work on all Linuxes.

I've not looked into Windows yet.
Attachment #8963903 - Flags: review?(enndeakin)
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #15)
> I've not looked into Windows yet.

scrollbar width on Windows 10 is 17px; the current 15px image looks alright there.
Comment hidden (mozreview-request)
(Reporter)

Comment 18

11 months ago
mozreview-review
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

https://reviewboard.mozilla.org/r/232760/#review238474

::: commit-message-fd862:1
(Diff revision 4)
> +Bug 1450017 - Follow-up, Move resizer style rules to minimal-xml.css

Nit: minimal-xul.css
Comment hidden (mozreview-request)
Thanks :bgrins for pointing this out. There are a few reftests that should have failed but it passes on Try.

Specifically, any comparison involves this one testing native theme:

https://searchfox.org/mozilla-central/source/layout/reftests/native-theme/resizer-bottomend-rtl.xul

Given that I moved the code that sets the style of this (style="direction: rtl") resizer away, this should end up having the same look as resizer-bottomend.xul. But I can see it passes here: https://treeherder.mozilla.org/#/jobs?repo=try&author=timdream@gmail.com&selectedJob=171464709 I am spinning up Linux local build to inspect locally but that would take some time.

Also, I am still struggling to find an example where we use native-themed resizer to verify if it still looks alright. If it's not used anywhere, I wonder if we should still support it.

The other thing worth considering removing is dir attribute value "bottomend" and "bottomstart". They are RTL-aware version of "bottomright" and "bottomleft". The code I've moved away from XBL is the bit that detects RTL.

Neil, could you help me understand this? Do we use native-themed resizer anywhere? What's your opinion on dropping "bottomend" and "bottomstart"?
Flags: needinfo?(enndeakin)

Comment 21

11 months ago
(In reply to Brian Grinstead [:bgrins] from comment #9)
> :shorlander, can you help us get svg version(s) of the resizer icons used
> for textareas? As you mentioned we may not need to keep the three OS
> variants, but if we do end up wanting to it'd be fine as well.
> 
> *
> https://searchfox.org/mozilla-central/source/toolkit/themes/linux/global/
> icons/resizer.png
> *
> https://searchfox.org/mozilla-central/source/toolkit/themes/windows/global/
> icons/resizer.png
> *
> https://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/icons/
> resizer.png
> *
> https://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/icons/
> resizer@2x.png

The responsive design mode has some SVG resizers if that's what you're looking for: https://dxr.mozilla.org/mozilla-central/source/devtools/client/responsive.html/images/grippers.svg
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #20)
> Specifically, any comparison involves this one testing native theme:
> 
> https://searchfox.org/mozilla-central/source/layout/reftests/native-theme/
> resizer-bottomend-rtl.xul

I've manually inspected the xul dialog rendered from this file on Linux and Mac. I can see the <resizer> is still flipped with my patch.

I can also make the test fail by removing `-moz-appearance: resizer;` in minimal-xul.css. So somewhere in the native theme handling must have checked the direction too and act accordingly, independent of the XBL code that I've moved.

I guess the question here now is that whether or not we accept the fact of NOT having non-natively-themed, non-scroll-frame <resizer>s handle the CSS direction property, given there is no use case nor tests for this feature.

Comment 23

11 months ago
> Neil, could you help me understand this? Do we use native-themed resizer
> anywhere? What's your opinion on dropping "bottomend" and "bottomstart"?

Cases such as 418864 will want a native resizer that isn't in a scrollable area, so I don't think it should be removed.
Flags: needinfo?(enndeakin)
Comment on attachment 8963903 [details]
Bug 1450017 - Part VI, Remove the resizer binding

Understood. In that case, I will look for the alternative approach.
Attachment #8963903 - Flags: review?(enndeakin)
(In reply to Neil Deakin from comment #23)
> Cases such as 418864 will want a native resizer that isn't in a scrollable
> area, so I don't think it should be removed.

OK. My current theory is that we can replace the feature (awareness of |direction: rtl|) with the existing |:-moz-locale-dir(rtl)| selector. Is that a right assumption? I will create another set of reftests to test non-native-themed resizers first.
More context: I have been trying to see whether or not I could set the attribute value at nsXULElement::BindToTree(), but it seems that the RTL information I can get there will be from XULDocument::IsDocumentRightToLeft(), which is the same locale-dir accessible from CSS.

The other thing is that |direction: rtl| itself, in non-reftest XUL docs, is set by matching |:root:-moz-locale-dir(rtl)|, in minimal-xul.css. It doesn't make sense to me do what we currently do, i.e. selector |locale-dir(rtl)| matches -> |direction: rtl| set -> read CSS computed value -> set an attribute -> cause more CSS selector matches.

To sum, this is my current plan:

1 For resizers in the scrollbar, set |bottomright| or |bottomleft| outright, instead of |bottomend|.
2 For standalone resizer, 
2.1 Verify that non-native-themed resizer flips when |:-moz-locale-dir(rtl)| matches.
2.2 Make sure native-themed resizer continue to flip when |:-moz-locale-dir(rtl)| matches too.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 30

11 months ago
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #25)
> OK. My current theory is that we can replace the feature (awareness of
> |direction: rtl|) with the existing |:-moz-locale-dir(rtl)| selector. Is
> that a right assumption? I will create another set of reftests to test
> non-native-themed resizers first.

direction:rtl will be applied when an element is right-to-left whereas -moz-locale-dir(rtl) matches when the user agent is configured to be rtl.
(In reply to Neil Deakin from comment #30)
> direction:rtl will be applied when an element is right-to-left whereas
> -moz-locale-dir(rtl) matches when the user agent is configured to be rtl.

What's the difference between two on XUL windows? Is there really a use case of direction:rtl resizer on non-RTL locale?

Comment 32

11 months ago
Comment on attachment 8963903 [details]
Bug 1450017 - Part VI, Remove the resizer binding

After looking at this, I don't think I understand what is happening here enough to be able to review this.
Attachment #8963903 - Flags: review?(enndeakin)
Could you help me with comment 31? I would like to understand what's needed before moving forward.
Flags: needinfo?(enndeakin)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8966802 - Flags: review?(enndeakin)
Attachment #8965010 - Flags: review?(enndeakin)
Attachment #8966803 - Flags: review?(enndeakin)
Attachment #8966802 - Flags: review?(enndeakin)
Attachment #8965010 - Flags: review?(enndeakin)
Attachment #8966803 - Flags: review?(enndeakin)
The changesets posted here is somewhat the same patch; I just expand them so it's easier to understand step-by-step, hopefully, make it easier to review.

The Linux resizer-rtl.png bug is a bit unfortunate; thankfully it's caught with the new reftests. The end result of the patches again is the removal of resizer.xml and the maintenance of support of standalone XUL <resizer> and <resizer> in nsGfxScrollFrame, particularly on different directions and the context-aware values (bottomend/start).

Depend on the available cycles we get from UX we could decide not to check-in the last patch in this bug.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8966804 - Flags: review?(enndeakin)
Attachment #8966805 - Flags: review?(enndeakin)
Attachment #8963903 - Flags: review?(enndeakin)
Attachment #8966806 - Flags: review?(enndeakin)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 52

11 months ago
mozreview-review
Comment on attachment 8965010 [details]
Bug 1450017 - Part II, Reftest for non-native-theme resizer

https://reviewboard.mozilla.org/r/233752/#review241452
Attachment #8965010 - Flags: review?(enndeakin) → review+

Updated

11 months ago
Flags: needinfo?(enndeakin)
Attachment #8965010 - Flags: review?(enndeakin)

Updated

11 months ago
Attachment #8966802 - Flags: review?(enndeakin)

Updated

11 months ago
Attachment #8966803 - Flags: review?(enndeakin)

Comment 53

11 months ago
mozreview-review
Comment on attachment 8966802 [details]
Bug 1450017 - Part I, Package resizer-rtl.png correctly on Linux

https://reviewboard.mozilla.org/r/235482/#review241454
Attachment #8966802 - Flags: review?(enndeakin) → review+
Comment on attachment 8966803 [details]
Bug 1450017 - Part III, Replace direction: rtl with localedir="rtl" in resizer reftests

https://reviewboard.mozilla.org/r/235484/#review241456

::: commit-message-6ff11:1
(Diff revision 1)
> +Bug 1450017 - Replace direction: rtl with chromedir="rtl" in resizer reftests

localedir

Comment 55

11 months ago
mozreview-review
Comment on attachment 8966806 [details]
Bug 1450017 - Part VII, Remove unused CSS rules in macOS resizer.css

https://reviewboard.mozilla.org/r/235490/#review241462
Attachment #8966806 - Flags: review?(enndeakin) → review+

Comment 56

11 months ago
mozreview-review
Comment on attachment 8966804 [details]
Bug 1450017 - Part IV, Always set resizer direction explicitly in ScrollFrameHelper::CreateAnonymousContent()

https://reviewboard.mozilla.org/r/235486/#review241466
Attachment #8966804 - Flags: review?(enndeakin) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Comment 63

10 months ago
Created attachment 8968937 [details]
resizer.svg

Here is the SVG for the resizer (same icon for all platforms). From shorlander: "We should be able to use this one everywhere. Let me know if it works."
(Reporter)

Updated

10 months ago
Flags: needinfo?(shorlander)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Updated

10 months ago
Priority: -- → P5
Comment hidden (mozreview-request)

Comment 73

10 months ago
mozreview-review
Comment on attachment 8966803 [details]
Bug 1450017 - Part III, Replace direction: rtl with localedir="rtl" in resizer reftests

https://reviewboard.mozilla.org/r/235484/#review245018
Attachment #8966803 - Flags: review?(enndeakin) → review+

Comment 74

10 months ago
mozreview-review
Comment on attachment 8963903 [details]
Bug 1450017 - Part VI, Remove the resizer binding

https://reviewboard.mozilla.org/r/232758/#review245020
Attachment #8963903 - Flags: review?(enndeakin) → review+

Comment 75

10 months ago
mozreview-review
Comment on attachment 8966805 [details]
Bug 1450017 - Part V, Remove rtl="true" attribute from resizer

https://reviewboard.mozilla.org/r/235488/#review245024
Attachment #8966805 - Flags: review?(enndeakin) → review+

Comment 76

10 months ago
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

Dao is a better reviewer for style changes.
Attachment #8963904 - Flags: review?(enndeakin) → review?(dao+bmo)
The last changeset fails dom/base/test/browser_use_counters.js, which counts SVG property usages.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d883312cbc9230219b25e9e5d066cc6705ffad76&selectedJob=175364179

Strangely it only fails on Windows. Perhaps the resizer only shows up there? I don't have a Windows setup to see what's visually shown when the test is run.

Regardless, rather than fixing the tests, I think there should be some filter in-place so we don't account feature usage from chrome:// SVG images (not the URL scheme of the document). Casually looking at relevant code changes made in bug 968923, I didn't find an obvious place to do that. Let me think about it...

Comment 78

10 months ago
mozreview-review
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

https://reviewboard.mozilla.org/r/232760/#review245118

::: layout/reftests/xul/reftest.list:87
(Diff revision 12)
>  # resizer (non-native-themed)
>  != resizer-bottomend.xul blank-window.xul
>  == resizer-bottomend.xul resizer-bottomright.xul
>  != resizer-bottomend.xul resizer-bottomend-rtl.xul
>  != resizer-bottomend-rtl.xul blank-window.xul
> -== resizer-bottomend-rtl.xul resizer-bottomend-flipped.xul
> +fuzzy(42,98) == resizer-bottomend-rtl.xul resizer-bottomend-flipped.xul # SVG difference from CSS-flipped to the RTL image.

How did this work before this patch? Why is fuzzy the right way to deal with this?
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

https://reviewboard.mozilla.org/r/232760/#review245118

> How did this work before this patch? Why is fuzzy the right way to deal with this?

Because before this patch we were using raster images, CSS-flipped image can be prefectly matched to a flipped image. A flipped SVG won't match with that.

The purpose of these tests was to make sure the resizer points to the right edges. fuzzy() with limited maxPixelCount and maxDiff can still confirm it.
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #77)
> Regardless, rather than fixing the tests, I think there should be some
> filter in-place so we don't account feature usage from chrome:// SVG images
> (not the URL scheme of the document). Casually looking at relevant code
> changes made in bug 968923, I didn't find an obvious place to do that. Let
> me think about it...

Got a patch on Try to do this; let's see if I am doing this right ...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b8b825448f22962e4fbb71957aa4686ac0ed913

Comment 81

10 months ago
mozreview-review-reply
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

https://reviewboard.mozilla.org/r/232760/#review245118

> Because before this patch we were using raster images, CSS-flipped image can be prefectly matched to a flipped image. A flipped SVG won't match with that.
> 
> The purpose of these tests was to make sure the resizer points to the right edges. fuzzy() with limited maxPixelCount and maxDiff can still confirm it.

Can you make this more explicit in the comment? "SVG difference from CSS-flipped to the RTL image" isn't really helpful.

Comment 82

10 months ago
mozreview-review
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

https://reviewboard.mozilla.org/r/232760/#review245128
Attachment #8963904 - Flags: review?(dao+bmo) → review+

Comment 83

10 months ago
mozreview-review
Comment on attachment 8963904 [details]
Bug 1450017 - Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style

https://reviewboard.mozilla.org/r/232760/#review245158

::: toolkit/content/minimal-xul.css:91
(Diff revision 12)
> +
> +/* Do not try to remove the RTL asset and flip it with CSS transform;
> +   native theme style rely on CSS transfrom below to flip too, when
> +   replacing the background image. */
> +resizer:-moz-locale-dir(rtl) {
> +  background: url("chrome://global/skin/icons/resizer-rtl.svg") no-repeat;

We could probably do something like `background: url("chrome://global/skin/icons/resizer.svg#rtl")` and doing x-flip inside an inline `<style>` in the SVG when the URI fragment is set to `#rtl`, similarly to how the Multi-Account Cointaners extension [chooses the container icon](https://github.com/mozilla/multi-account-containers/blob/master/src/img/usercontext.svg?short_path=f58067a).
(In reply to ExE Boss from comment #83)

That's a great idea, thanks!

Comment 85

10 months ago
(In reply to ExE Boss from comment #83)
> We could probably do something like `background:
> url("chrome://global/skin/icons/resizer.svg#rtl")` and doing x-flip inside
> an inline `<style>` in the SVG when the URI fragment is set to `#rtl`,
> similarly to how the Multi-Account Cointaners extension [chooses the
> container
> icon](https://github.com/mozilla/multi-account-containers/blob/master/src/
> img/usercontext.svg?short_path=f58067a).

Please do not this hack, we're trying to get rid of it in Firefox for better performance, see bug 1358998.

Comment 86

10 months ago
(In reply to Tim Nguyen :ntim from comment #85)
> (In reply to ExE Boss from comment #83)
> > We could probably do something like `background:
> > url("chrome://global/skin/icons/resizer.svg#rtl")` and doing x-flip inside
> > an inline `<style>` in the SVG when the URI fragment is set to `#rtl`,
> > similarly to how the Multi-Account Cointaners extension [chooses the
> > container
> > icon](https://github.com/mozilla/multi-account-containers/blob/master/src/
> > img/usercontext.svg?short_path=f58067a).
> 
> Please do not this hack, we're trying to get rid of it in Firefox for better
> performance, see bug 1358998.

I meant something more along the lines of:

```svg
<svg>
	<style>
		#rtl:target {
			/* X-flip the SVG */
		}
	</style>
	<g id="rtl">
		<!-- SVG content -->
	</g>
</svg>
```

Comment 87

10 months ago
(In reply to ExE Boss from comment #86)


While your approach saves 1 file, it still has the same issues described in bug 1358998 comment 0 though.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 99

10 months ago
mozreview-review
Comment on attachment 8971029 [details]
Bug 1450017 - Part VIII, Exclude chrome UA style images from use counters

https://reviewboard.mozilla.org/r/239750/#review245550

This is OK but it might be good to keep the code that makes decisions about use counter reporting in the one place.  So how about we put the check in nsIDocument::PropagateUseCounters, and then you can either call the existing MightBeAboutOrChromeScheme function, or split it up into a separate MightBeChromeScheme that you can call from MightBeAboutOrChromeScheme and PropagateUseCounters.  r=me with that.
Attachment #8971029 - Flags: review?(cam) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 104

10 months ago
Pushed by timdream@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/dd262c337f19
Part I, Package resizer-rtl.png correctly on Linux r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/2d1c525b5404
Part II, Reftest for non-native-theme resizer r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/4ffb62df7a59
Part III, Replace direction: rtl with localedir="rtl" in resizer reftests r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/407774fc7c7a
Part IV, Always set resizer direction explicitly in ScrollFrameHelper::CreateAnonymousContent() r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/eabdcd541092
Part V, Remove rtl="true" attribute from resizer r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/d50230b89a1e
Part VI, Remove the resizer binding r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/30abf100e117
Part VII, Remove unused CSS rules in macOS resizer.css r=enndeakin+6102
https://hg.mozilla.org/integration/autoland/rev/7d7480f8fd9e
Part VIII, Exclude chrome UA style images from use counters r=heycam
https://hg.mozilla.org/integration/autoland/rev/94d0e43dad11
Part IX, Move resizer style rules to minimal-xul.css and unify non-native resizer style r=dao

Updated

10 months ago
Depends on: 1459646
You need to log in before you can comment on or make changes to this bug.