Closed Bug 1697863 Opened 2 years ago Closed 2 years ago

Add preliminary Proton styles for browser UI menulists

Tracking

()

Status:

RESOLVED FIXED

Milestone:

88 Branch

Tracking Flags:

Tracking

Status

firefox88

---

fixed

People

(Reporter: mconley, Assigned: mconley)

References

(Blocks 1 open bug, Regressed 2 open bugs)

Details

(Whiteboard: [proton-door-hangers])

Attachments

(3 files)

Bug 1697863 - Add preliminary Proton styles for browser UI menulists. r?harry! 2 years ago Mike Conley (:mconley) (:⚙️) 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1697863 - Use native menulist styling on some dialogs. r?harry! 2 years ago Mike Conley (:mconley) (:⚙️) 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1697863 - Add another Stylo valgrind suppression. r?emilio! 2 years ago Mike Conley (:mconley) (:⚙️) 48 bytes, text/x-phabricator-request		Details \| Review

Mike Conley (:mconley) (:⚙️)

Assignee

Description

•

2 years ago

Similar to bug 1697315. For Proton, we're changing the look and feel of browser UI menulists. This patch introduces the new styling, with a legacy attribute that can be set on the menulist to fallback to the old styling if we need to.

Mike Conley (:mconley) (:⚙️)

Assignee

Comment 1

•

2 years ago

Attached file Bug 1697863 - Add preliminary Proton styles for browser UI menulists. r?harry! — Details

Jira Integration Bot

Updated

•

2 years ago

See Also: → https://jira.mozilla.com/browse/FIDEFE-497

Mike Conley (:mconley) (:⚙️)

Assignee

Comment 2

•

2 years ago

Attached file Bug 1697863 - Use native menulist styling on some dialogs. r?harry! — Details

Depends on D108061

Pulsebot

Comment 3

•

2 years ago

Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/64166dc7f85f
Add preliminary Proton styles for browser UI menulists. r=harry,desktop-theme-reviewers
https://hg.mozilla.org/integration/autoland/rev/f82846b236e3
Use native menulist styling on some dialogs. r=harry

Natalia Csoregi [:nataliaCs]

Comment 4

•

2 years ago

Backed out for valgrind failures

backout: https://hg.mozilla.org/integration/autoland/rev/6834c693610e056902f6729f8a0554994c0c1272

push: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=linux%2Cx64%2Copt%2Cvalgrind-linux64-valgrind%2Fopt%2Cv&revision=f82846b236e3e5056988793f66e5c69dc6127fee

failure log: https://treeherder.mozilla.org/logviewer?job_id=333450892&repo=autoland&lineNumber=60856

[task 2021-03-16T22:34:04.603Z] 22:34:04     INFO -   3:12.21 TEST-UNEXPECTED-FAIL | valgrind-test | Conditional jump or move depends on uninitialised value(s) at selectors::parser::parse_one_simple_selector / selectors::parser::parse_selector / selectors::parser::parse_functional_pseudo_class / selectors::parser::parse_one_simple_selector
[task 2021-03-16T22:34:04.603Z] 22:34:04     INFO -   3:12.21 ==12371== Conditional jump or move depends on uninitialised value(s)
[task 2021-03-16T22:34:04.603Z] 22:34:04     INFO -   3:12.21 ==12371==    at 0x15D35769: selectors::parser::parse_one_simple_selector+7817 (parser.rs:2023)
[task 2021-03-16T22:34:04.603Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15D303D7: selectors::parser::parse_selector+1175 (parser.rs:2130)
[task 2021-03-16T22:34:04.603Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15D39257: selectors::parser::parse_functional_pseudo_class+1399 (parser.rs:417)
[task 2021-03-16T22:34:04.604Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15D35031: selectors::parser::parse_one_simple_selector+5969 (parser.rs:2417)
[task 2021-03-16T22:34:04.604Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15D303D7: selectors::parser::parse_selector+1175 (parser.rs:2130)
[task 2021-03-16T22:34:04.604Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15D2F7A8: selectors::parser::SelectorList<Impl>::parse_with_state+120 (parser.rs:377)
[task 2021-03-16T22:34:04.604Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15E355D3: style::stylesheets::rule_parser::NestedRuleParser::parse_nested_rules+2483 (parser.rs:356)
[task 2021-03-16T22:34:04.604Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15E3240A: <style::stylesheets::rule_parser::NestedRuleParser as cssparser::rules_and_declarations::AtRuleParser>::parse_block+490 (servo/components/style/stylesheets/rule_parser.rs:525)
[task 2021-03-16T22:34:04.604Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15E30DF0: cssparser::rules_and_declarations::parse_at_rule+5280 (servo/components/style/stylesheets/rule_parser.rs:257)
[task 2021-03-16T22:34:04.605Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x15E4B00C: style::stylesheets::stylesheet::StylesheetContents::from_str+1980 (rules_and_declarations.rs:391)
[task 2021-03-16T22:34:04.605Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x157B6A56: Servo_StyleSheet_FromUTF8Bytes+454 (servo/ports/geckolib/glue.rs:1565)
[task 2021-03-16T22:34:04.605Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x13A31AF8: mozilla::StyleSheet::ParseSheet(mozilla::css::Loader&, nsTSubstring<char> const&, mozilla::css::SheetLoadData&)+472 (checkouts/gecko/layout/style/StyleSheet.cpp:1187)
[task 2021-03-16T22:34:04.605Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x13A1F971: mozilla::css::Loader::ParseSheet(nsTSubstring<char> const&, mozilla::css::SheetLoadData&, mozilla::css::Loader::AllowAsyncParse)+321 (checkouts/gecko/layout/style/Loader.cpp:1522)
[task 2021-03-16T22:34:04.605Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x13A2DEC2: mozilla::css::StreamLoader::OnStopRequest(nsIRequest*, nsresult)+562 (checkouts/gecko/layout/style/StreamLoader.cpp:143)
[task 2021-03-16T22:34:04.606Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x110EBAF6: nsBaseChannel::OnStopRequest(nsIRequest*, nsresult)+118 (checkouts/gecko/netwerk/base/nsBaseChannel.cpp:849)
[task 2021-03-16T22:34:04.606Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x110EBBBC: non-virtual thunk to nsBaseChannel::OnStopRequest(nsIRequest*, nsresult)+12 (checkouts/gecko/netwerk/base/nsBaseChannel.cpp:0)
[task 2021-03-16T22:34:04.606Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11104398: nsInputStreamPump::OnStateStop()+328 (checkouts/gecko/netwerk/base/nsInputStreamPump.cpp:649)
[task 2021-03-16T22:34:04.606Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11103C0A: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*)+426 (checkouts/gecko/netwerk/base/nsInputStreamPump.cpp:397)
[task 2021-03-16T22:34:04.606Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x10FF8363: nsInputStreamReadyEvent::Run()+35 (checkouts/gecko/xpcom/io/nsStreamUtils.cpp:94)
[task 2021-03-16T22:34:04.606Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11017C03: mozilla::RunnableTask::Run()+195 (checkouts/gecko/xpcom/threads/TaskController.cpp:472)
[task 2021-03-16T22:34:04.607Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11016B41: mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&)+1617 (checkouts/gecko/xpcom/threads/TaskController.cpp:760)
[task 2021-03-16T22:34:04.607Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11015E34: mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&)+36 (checkouts/gecko/xpcom/threads/TaskController.cpp:611)
[task 2021-03-16T22:34:04.607Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11015FC7: mozilla::TaskController::ProcessPendingMTTask(bool)+55 (checkouts/gecko/xpcom/threads/TaskController.cpp:395)
[task 2021-03-16T22:34:04.607Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11019771: operator() (checkouts/gecko/xpcom/threads/TaskController.cpp:133)
[task 2021-03-16T22:34:04.607Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11019771: mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run()+17 (checkouts/gecko/xpcom/threads/nsThreadUtils.h:534)
[task 2021-03-16T22:34:04.608Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11023DF7: nsThread::ProcessNextEvent(bool, bool*)+1623 (checkouts/gecko/xpcom/threads/nsThread.cpp:1158)
[task 2021-03-16T22:34:04.608Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11027D87: NS_ProcessNextEvent(nsIThread*, bool)+71 (checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:548)
[task 2021-03-16T22:34:04.608Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x115CF987: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)+135 (checkouts/gecko/ipc/glue/MessagePump.cpp:87)
[task 2021-03-16T22:34:04.608Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11589B35: RunInternal (checkouts/gecko/ipc/chromium/src/base/message_loop.cc:335)
[task 2021-03-16T22:34:04.608Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11589B35: RunHandler (checkouts/gecko/ipc/chromium/src/base/message_loop.cc:328)
[task 2021-03-16T22:34:04.608Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x11589B35: MessageLoop::Run()+85 (checkouts/gecko/ipc/chromium/src/base/message_loop.cc:310)
[task 2021-03-16T22:34:04.609Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x1388E618: nsBaseAppShell::Run()+40 (checkouts/gecko/widget/nsBaseAppShell.cpp:137)
[task 2021-03-16T22:34:04.609Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x14A19716: nsAppStartup::Run()+118 (checkouts/gecko/toolkit/components/startup/nsAppStartup.cpp:273)
[task 2021-03-16T22:34:04.609Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x14AE904B: XREMain::XRE_mainRun()+3051 (checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5352)
[task 2021-03-16T22:34:04.609Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x14AE998C: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)+1164 (checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5543)
[task 2021-03-16T22:34:04.609Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x14AE9D4E: XRE_main(int, char**, mozilla::BootstrapConfig const&)+158 (checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5606)
[task 2021-03-16T22:34:04.609Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x12ACB3: do_main (checkouts/gecko/browser/app/nsBrowserApp.cpp:220)
[task 2021-03-16T22:34:04.610Z] 22:34:04     INFO -   3:12.21 ==12371==    by 0x12ACB3: main+1203 (checkouts/gecko/browser/app/nsBrowserApp.cpp:347)
[task 2021-03-16T22:34:04.610Z] 22:34:04     INFO -   3:12.21 ==12371==  Uninitialised value was created by a stack allocation
[task 2021-03-16T22:34:04.610Z] 22:34:04     INFO -   3:12.21 ==12371==    at 0x15D338ED: selectors::parser::parse_one_simple_selector+13 (parser.rs:2306)
[task 2021-03-16T22:34:04.610Z] 22:34:04     INFO -   3:12.21 ==12371==

Flags: needinfo?(mconley)

Mike Conley (:mconley) (:⚙️)

Assignee

Comment 5

•

2 years ago

Huh... I'm kind of at a loss to explain this one... emilio, any idea what I've tripped here?

Flags: needinfo?(mconley) → needinfo?(emilio)

Emilio Cobos Álvarez (:emilio)

Comment 6

•

2 years ago

This is a known Valgrind false-positive.

This seems a version of this suppression just with an slightly different stack.

In particular, it looks like this suppression but without the parse_until_before stack frame.

I don't know if valgrind uses PGO builds. If it does, you probably made a code-path frequent enough that the compiler decided to inline parse_until_before.

In any case what needs to happen is adjusting the Valgrind suppression list, either by removing the line linked above, or adding a suppression for a stack without that line.

Flags: needinfo?(emilio)

Mike Conley (:mconley) (:⚙️)

Assignee

Comment 7

•

2 years ago

Attached file Bug 1697863 - Add another Stylo valgrind suppression. r?emilio! — Details

Depends on D108101

Mike Conley (:mconley) (:⚙️)

Assignee

Comment 8

•

2 years ago

Here's a trybuild with the new suppression, which valgrind seems to like: https://treeherder.mozilla.org/jobs?repo=try&revision=008f1d9868960fc3606c6fc6c17b0d9069cada64

Pulsebot

Comment 9

•

2 years ago

Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6f19f39e7c5
Add preliminary Proton styles for browser UI menulists. r=harry,desktop-theme-reviewers
https://hg.mozilla.org/integration/autoland/rev/bad811dda7ec
Use native menulist styling on some dialogs. r=harry
https://hg.mozilla.org/integration/autoland/rev/42f6412b7bc1
Add another Stylo valgrind suppression. r=emilio

Bogdan Tara[:bogdan_tara | bogdant]

Comment 10

•

2 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/d6f19f39e7c5
https://hg.mozilla.org/mozilla-central/rev/bad811dda7ec
https://hg.mozilla.org/mozilla-central/rev/42f6412b7bc1

Status: NEW → RESOLVED

Closed: 2 years ago

status-firefox88: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 88 Branch

Tim Nguyen :ntim

Updated

•

2 years ago

Regressions: 1699553

Asif Youssuff

Updated

•

2 years ago

Regressions: 1700464

Bogdan Maris [:bogdan_maris], Desktop QA

Updated

•

2 years ago

Regressions: 1703823

Alice0775 White

Updated

•

2 years ago

Regressions: 1710836

You need to log in before you can comment on or make changes to this bug.

Bugzilla

Quick Search

Add preliminary Proton styles for browser UI menulists

Categories

(Firefox :: Theme, task)

Tracking

()

People

(Reporter: mconley, Assigned: mconley)

References

(Blocks 1 open bug, Regressed 2 open bugs)

Details

(Whiteboard: [proton-door-hangers])

Crash Data

Security

(public)

User Story

Attachments

(3 files)

Description

Comment 1

Updated

Comment 2

Comment 3

Comment 4

Comment 5

Comment 6

Comment 7

Comment 8

Comment 9

Comment 10

Updated

Updated

Updated

Updated