Page MenuHomePhabricator

Bug 1601990 : convert NS_STYLE_POINTER_EVENTS_* to an enum class in nsStyleConsts.h r=emilio
ClosedPublic

Authored by jeffin143 on Dec 6 2019, 3:46 PM.

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Build Status
Buildable 145970
Build 219886: arc lint + arc unit

Event Timeline

jeffin143 created this revision.Dec 6 2019, 3:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2019, 3:46 PM
phab-bot requested review of this revision.Dec 6 2019, 3:46 PM
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: Restricted Project.

@emilio Need your help, couldn't understand the error !!

The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.

If you see a problem in this automated review, please report it here.

emilio added a comment.Dec 6 2019, 4:15 PM

@emilio Need your help, couldn't understand the error !!

Can you paste it somewhere?

@emilio

https://treeherder.mozilla.org/#/jobs?repo=try&revision=248ff1227c0fbba45c8d0ea804db2c721d7e6300

error[E0599]: no variant or associated item named `Visiblepainted` found for type `gecko_bindings::structs::root::mozilla::StylePointerEvents` in the current scope
error[E0599]: no variant or associated item named `Visiblefill` found for type `gecko_bindings::structs::root::mozilla::StylePointerEvents` in the current scope
error[E0599]: no variant or associated item named `Visiblestroke` found for type `gecko_bindings::structs::root::mozilla::StylePointerEvents` in the current scope
error[E0599]: no variant or associated item named `Visiblepainted` found for type `gecko_bindings::structs::root::mozilla::StylePointerEvents` in the current scope
error[E0599]: no variant or associated item named `Visiblefill` found for type `gecko_bindings::structs::root::mozilla::StylePointerEvents` in the current scope
error[E0599]: no variant or associated item named `Visiblestroke` found for type `gecko_bindings::structs::root::mozilla::StylePointerEvents` in the current scope
error: Could not compile `style`.

    1584045Intermittent Win 2012 [taskcluster:error] Aborting task... | failed to execute compile | error: Could not compile `style`.

make[1]: *** [force-cargo-test-run] Error 101
make: *** [toolkit/library/rust/rusttests] Error 2
Return code: 2
'mach build -v pre-export export recurse_rusttests' did not run successfully. Please check log for errors.
Running post_fatal callback...
emilio added inline comments.Dec 6 2019, 4:44 PM
layout/style/nsStyleConsts.h
573

Well, you need to rename these to match the css keyword.

The keyword is visiblepainted, so the enum is Visiblepainted, not VisiblePainted (which would be the case for visible-painted, for example).

jeffin143 updated this revision to Diff 204451.Dec 7 2019, 11:08 AM

The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.

If you see a problem in this automated review, please report it here.

emilio requested changes to this revision.Dec 8 2019, 10:58 AM
emilio added inline comments.
layout/style/nsStyleConsts.h
573

Instead, this should be Visiblepainted.

574

This should be Visiblefill.

575

And this Visiblestroke.

servo/components/style/properties/longhands/inherited_ui.mako.rs
27

No, you need to change the name of the enum variant, not this. This changes the values parsed, which is wrong.

This revision now requires changes to proceed.Dec 8 2019, 10:58 AM
ntim removed a subscriber: ntim.Dec 8 2019, 11:46 AM
jeffin143 updated this revision to Diff 205860.Dec 11 2019, 7:22 AM

The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.

If you see a problem in this automated review, please report it here.

emilio accepted this revision.Dec 11 2019, 2:29 PM

Thanks, this looks great!

This revision is now accepted and ready to land.Dec 11 2019, 2:29 PM
jeffin143 updated this revision to Diff 206127.Dec 11 2019, 3:38 PM
jeffin143 updated this revision to Diff 206139.Dec 11 2019, 4:16 PM
jeffin143 added inline comments.Dec 12 2019, 2:21 AM
layout/xul/nsMenuPopupFrame.cpp
289

@emilio /builds/worker/workspace/build/src/layout/xul/nsMenuPopupFrame.cpp:288:75: error: expected unqualified-id

should i wrap it in parenthesis , I am not sure did I overlook something

Can you paste the whole error message?

/builds/worker/workspace/build/src/layout/xul/nsMenuPopupFrame.cpp:288:75: error: expected unqualified-id
/builds/worker/workspace/build/src/layout/xul/nsMenuPopupFrame.cpp:480:73: error: expected unqualified-id
/builds/worker/workspace/build/src/layout/xul/nsMenuPopupFrame.cpp:482:63: error: expected unqualified-id
make[4]: *** [Unified_cpp_layout_xul0.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [layout/xul/target-objects] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [compile] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2

1515621Intermittent make: *** [build] Error 2 due to broken artifact builds on Android 7.0 (missing app-withoutGeckoBinaries-debug.apk)
1442211Intermittent make: *** [build] Error 2 after Starting sccache server... error: Timed out waiting for server startup
1570503Intermittent make: *** [build] Error 2 after [js/src/jsapi-tests/target] Error 2

    1470945Intermittent make: *** [build] Error 2 after error reading compile response from server

Return code: 2
'mach build -v' did not run successfully. Please check log for errors.

1584213Intermittent 'mach build -v' did not run successfully. Please check log for errors. after [client.mk:125: build] Error 2

    1598844Intermittent 'mach build -v' did not run successfully. Please check log for errors. after KeyError: u'package-generated-sources'

Show / Hide more
Running post_fatal callback...
Exiting -1
# TBPL FAILURE #
# TBPL FAILURE #

@emilio Here is the job - https://treeherder.mozilla.org/#/jobs?repo=try&revision=b65f3d43b4a44d1dd223fc04da3d912349446487&selectedJob=280783932

Thanks. This is an X11 header which has a macro called None defined to be 0. From the log:

[task 2019-12-12T01:57:24.903Z] 01:57:24 INFO - /usr/include/X11/X.h:115:30: note: expanded from macro 'None'
[task 2019-12-12T01:57:24.903Z] 01:57:24 INFO - #define None 0L /* universal null resource or null atom */

So in practice that line ends up being:

StyleUI()->GetEffectivePointerEvents(this) == StylePointerEvents::0L;

Which fails to compile.

I would add:

#include "X11UndefineNone.h"

After this: https://searchfox.org/mozilla-central/rev/62a130ba0ac80f75175e4b65536290b52391f116/layout/xul/nsMenuPopupFrame.cpp#64

Which should, well, undefine None, and make the code build :)

jeffin143 updated this revision to Diff 206540.Dec 12 2019, 4:38 AM

@emilio Thanks for investigating and telling me the solution

Everything is green : - https://treeherder.mozilla.org/#/jobs?repo=try&revision=046903c7270e6a7bad996ac23aff20a1d1aedc17

I will check in this :)

lobot added a subscriber: lobot.

Check-in Needed handled, landing queued.