Page MenuHomePhorge

Unify user-select CSS directives
ClosedPublic

Authored by Leon95 on Oct 16 2021, 10:49.

Details

Summary

Removes the -khtml, -moz and -ms prefix, since most Browsers are natively supporting the user select directive.
The -webkit prefix is still kept or added for Safari, wich does not support user-select.

Ref: see https://we.phorge.it/D25024#815 for context

Test Plan

Removing the CSS should change nothing in modern browsers.

Diff Detail

Repository
rP Phorge
Branch
master
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 51
Build 51: arc lint + arc unit

Unit TestsFailed

TimeTest
92 msPhabricatorCelerityTestCase::testCelerityMaps
Assertion failed, expected 'true' (at PhabricatorCelerityTestCase.php:32): When this test fails, it means the Celerity resource map is out of date. Run `bin/celerity map` to rebuild it. ACTUAL VALUE
1 msPhabricatorAnchorTestCase::testAnchors
8 assertions passed.
13 msPhabricatorConduitTestCase::testConduitMethods
1 assertion passed.
0 msPhabricatorInfrastructureTestCase::testApplicationsInstalled
1 assertion passed.
1 msPhabricatorInfrastructureTestCase::testRejectMySQLNonUTF8Queries
1 assertion passed.
View Full Test Results (1 Failed · 21 Passed · 1 Skipped)

Event Timeline

Leon95 requested review of this revision.Oct 16 2021, 10:49

Thank you for going through to make these all consistent!

This revision is now accepted and ready to land.Oct 22 2021, 15:51

@Leon95 I encountered a merge conflict when attempting to land these changes. Could you please take a look and try to fix and land them yourself? If you need help, feel free to ping me.

Matthew requested changes to this revision.Oct 4 2022, 19:02
This revision now requires changes to proceed.Oct 4 2022, 19:02
valerio.bozzolan retitled this revision from Unify user-select css directives to Unify user-select CSS directives.Mar 27 2023, 20:01
valerio.bozzolan edited the test plan for this revision. (Show Details)
valerio.bozzolan added inline comments.
src/infrastructure/markup/rule/PhabricatorKeyboardRemarkupRule.php
215

Kept for Safari. Nice.

webroot/rsrc/css/application/base/standard-page-view.css
77

Kept for Safari. Nice.

webroot/rsrc/css/application/differential/changeset-view.css
436

The basic exists. Nice.

441

The basic exists. Nice.

451

The basic exists. Nice.

457

The basic exists. Nice.

webroot/rsrc/css/application/harbormaster/harbormaster.css
48

The basic exists. Nice.

webroot/rsrc/css/core/remarkup.css
70

The basic exists, and added for Safari. Nice.

webroot/rsrc/css/layout/phabricator-source-code-view.css
78

The basic exists. Nice.

webroot/rsrc/css/phui/button/phui-button.css
12

The basic exists. Nice.

webroot/rsrc/css/phui/object-item/phui-oi-list-view.css
679

The basic exists. Nice.

Probably this could be amended but I don't care.

webroot/rsrc/css/phui/workboards/phui-workcard.css
31

The basic case exists. Nice.

webroot/rsrc/css/phui/workboards/phui-workpanel.css
32

The basic case exista. Nice.

To make the unit test happy I strongly suggest to land D25133: Countdown: fix PhutilMissingSymbolException before working here. Otherwise we will get this nonsense error:

   FAIL  PhabricatorLibraryTestCase::testLibraryMap
Assertion failed, expected 'true' (at PhutilLibraryTestCase.php:51): The library map is out of date. Rebuild it with `arc liberate`.
These entries differ: class.PhabricatorCountdownCreateCapability, xmap.PhabricatorCountdownCreateCapability.

OK @Leon95 green light also from me. Hoping to be useful I've just rebased on master.

To land:

arc patch D25025
arc land

Thank you for this refactor

I encountered a merge conflict when attempting to land these changes. Could you please take a look and try to fix and land them yourself? If you need help, feel free to ping me.

Ah @Matthew I see you accepted this, but then marked as blocked for the old conflict (that now is probably resolved)

Hi @Matthew feel free to remove your red light here because of the merge conflict now solved (if you still like this change of course)

@Matthew please remove the red flag from this revision :) Thank you so much

Hoping to be useful I try to reset the Block from our kind Matthew since it was related to a past merge conflict

This revision is now accepted and ready to land.May 12 2023, 09:26

Hi @Leon95: green light :)

To land this:

arc patch D25025
arc land

Hoping to be useful I will land this approved change in 7 days if the original author can't :) Thanks again

Hoping to be useful I will land this approved change in 7 days if the original author can't :) Thanks again

As promise, I will land this in 30 minutes :) without more feedback. Thanks again!

Just to be sure, run again Celerity map:

./bin/celerity map

Then I will land, since this was approved by kind speck in Oct 2021, then kind Matthew tried to land in Sept 2022 ✨

This revision was automatically updated to reflect the committed changes.