Closed Bug 1389740 Opened 3 years ago Closed 3 years ago

Consolidate URL bar history dropmarker styling

Categories

(Firefox :: Theme, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 57
Iteration:
57.2 - Aug 29
Tracking Status
firefox57 --- fixed

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(1 file)

We have four different implementations of the history dropmarker: Linux, Windows, Mac, and compacttheme.css. Let's clean this up, both visually and implementation-wise.
Flags: qe-verify-
The patch may look messy at a first glance, because it does a bunch of things that I should probably have split up into multiple patches. The commit message contains an explanation of the different changes, I hope this helps.
Blocks: 1388589
Attachment #8896565 - Flags: review?(dharvey) → review?(gijskruitbosch+bugs)
Comment on attachment 8896565 [details]
Bug 1389740 - Consolidate URL bar history dropmarker styling.

https://reviewboard.mozilla.org/r/167828/#review173042

This lgtm. I do wonder how the -moz-box-align (which I do realize you're essentially just moving) is going to play out with having the highlight of the hover states cover the entire height of the navbar. Like, clearly we will need to come up with some way of setting the padding to exactly match, rather than using -moz-box-align: center... maybe something like: padding-top: calc(50% - 8px) ? Oh well, not really your problem here, I guess.

I also anticipate this will cause conflict with any work Drew is doing to reorder these items, but there's no real way around that either...
Attachment #8896565 - Flags: review?(gijskruitbosch+bugs) → review+
Iteration: --- → 57.1 - Aug 15
Priority: -- → P1
Whiteboard: [photon-visual] → [reserve-photon-visual]
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c73631b194bb
Consolidate URL bar history dropmarker styling. r=Gijs
Backed out for failing Firefox-UI's test_windows.py and test_about_private_browsing.py:

https://hg.mozilla.org/integration/autoland/rev/107b8c9fcd76829164580a7042e52bc230364fe1

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c73631b194bbedf4a20a7dcb9c648b25d178e7c5&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel

Failure log Firefox UI: https://treeherder.mozilla.org/logviewer.html#?job_id=122764687&repo=autoland
TEST-UNEXPECTED-ERROR | test_windows.py TestWindows.test_switch_to | TimeoutException: Timeout loading page after 300000ms
TEST-UNEXPECTED-ERROR | test_about_private_browsing.py TestAboutPrivateBrowsing.testCheckAboutPrivateBrowsing | NoSuchElementException: Unable to locate element: learnMore

There are also leaks on the push and the next one (sometimes they show up randomly, but hadn't seen it yet on a subsequent push, so it's likely caused by the patch)
Failure log leak: https://treeherder.mozilla.org/logviewer.html#?job_id=122764297&repo=autoland

[task 2017-08-12T16:28:17.287823Z] 16:28:16     INFO - GECKO(1107) | ==1218==WARNING: Can't read from symbolizer at fd 3
[task 2017-08-12T16:28:20.394888Z] 16:28:20     INFO - GECKO(1107) | LLVMSymbolizer: error reading file: Cannot allocate memory
[task 2017-08-12T16:28:20.652902Z] 16:28:20     INFO - GECKO(1107) | =================================================================
[task 2017-08-12T16:28:20.657211Z] 16:28:20    ERROR - GECKO(1107) | ==1218==ERROR: LeakSanitizer: detected memory leaks
[task 2017-08-12T16:28:20.677520Z] 16:28:20     INFO - GECKO(1107) | Direct leak of 15456 byte(s) in 483 object(s) allocated from:
[task 2017-08-12T16:28:20.685364Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.686892Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.688336Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a58fa299  (/home/worker/workspace/build/application/firefox/libxul.so+0x2166299)
[task 2017-08-12T16:28:20.690595Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a58f95ac  (/home/worker/workspace/build/application/firefox/libxul.so+0x21655ac)
[task 2017-08-12T16:28:20.691933Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a58f7a03  (/home/worker/workspace/build/application/firefox/libxul.so+0x2163a03)
[task 2017-08-12T16:28:20.702023Z] 16:28:20     INFO - GECKO(1107) | Direct leak of 4128 byte(s) in 129 object(s) allocated from:
[task 2017-08-12T16:28:20.704789Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.706132Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.709983Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a58fbd45  (/home/worker/workspace/build/application/firefox/libxul.so+0x2167d45)
[task 2017-08-12T16:28:20.715200Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a58eba40  (/home/worker/workspace/build/application/firefox/libxul.so+0x2157a40)
[task 2017-08-12T16:28:20.717448Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a58fae14  (/home/worker/workspace/build/application/firefox/libxul.so+0x2166e14)
[task 2017-08-12T16:28:20.722760Z] 16:28:20     INFO - GECKO(1107) | Direct leak of 1856 byte(s) in 58 object(s) allocated from:
[task 2017-08-12T16:28:20.729846Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.733469Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.741988Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a58fa299  (/home/worker/workspace/build/application/firefox/libxul.so+0x2166299)
[task 2017-08-12T16:28:20.745070Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a58f95ac  (/home/worker/workspace/build/application/firefox/libxul.so+0x21655ac)
[task 2017-08-12T16:28:20.754100Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a58f7955  (/home/worker/workspace/build/application/firefox/libxul.so+0x2163955)
[task 2017-08-12T16:28:20.757909Z] 16:28:20     INFO - GECKO(1107) | Direct leak of 480 byte(s) in 3 object(s) allocated from:
[task 2017-08-12T16:28:20.762247Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.766613Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.768512Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a7062572  (/home/worker/workspace/build/application/firefox/libxul.so+0x38ce572)
[task 2017-08-12T16:28:20.778097Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a58fe437  (/home/worker/workspace/build/application/firefox/libxul.so+0x216a437)
[task 2017-08-12T16:28:20.785822Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a58ff94d  (/home/worker/workspace/build/application/firefox/libxul.so+0x216b94d)
[task 2017-08-12T16:28:20.787724Z] 16:28:20     INFO - GECKO(1107) | Direct leak of 256 byte(s) in 8 object(s) allocated from:
[task 2017-08-12T16:28:20.790804Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.792690Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.794634Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a5900be9  (/home/worker/workspace/build/application/firefox/libxul.so+0x216cbe9)
[task 2017-08-12T16:28:20.803233Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a5966b91  (/home/worker/workspace/build/application/firefox/libxul.so+0x21d2b91)
[task 2017-08-12T16:28:20.805214Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a71712b0  (/home/worker/workspace/build/application/firefox/libxul.so+0x39dd2b0)
[task 2017-08-12T16:28:20.812280Z] 16:28:20     INFO - GECKO(1107) | Direct leak of 160 byte(s) in 1 object(s) allocated from:
[task 2017-08-12T16:28:20.822144Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.827417Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.829396Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a7062572  (/home/worker/workspace/build/application/firefox/libxul.so+0x38ce572)
[task 2017-08-12T16:28:20.833973Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a58fe437  (/home/worker/workspace/build/application/firefox/libxul.so+0x216a437)
[task 2017-08-12T16:28:20.835892Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a58ff4ed  (/home/worker/workspace/build/application/firefox/libxul.so+0x216b4ed)
[task 2017-08-12T16:28:20.839517Z] 16:28:20     INFO - GECKO(1107) | Indirect leak of 320 byte(s) in 8 object(s) allocated from:
[task 2017-08-12T16:28:20.841491Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.847588Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.849570Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a5900c76  (/home/worker/workspace/build/application/firefox/libxul.so+0x216cc76)
[task 2017-08-12T16:28:20.855015Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a5966b91  (/home/worker/workspace/build/application/firefox/libxul.so+0x21d2b91)
[task 2017-08-12T16:28:20.858194Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a71712b0  (/home/worker/workspace/build/application/firefox/libxul.so+0x39dd2b0)
[task 2017-08-12T16:28:20.867263Z] 16:28:20     INFO - GECKO(1107) | Indirect leak of 128 byte(s) in 8 object(s) allocated from:
[task 2017-08-12T16:28:20.871495Z] 16:28:20     INFO - GECKO(1107) |     #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-08-12T16:28:20.873388Z] 16:28:20     INFO - GECKO(1107) |     #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
[task 2017-08-12T16:28:20.875419Z] 16:28:20     INFO - GECKO(1107) |     #2 0x7fd6a5900c93  (/home/worker/workspace/build/application/firefox/libxul.so+0x216cc93)
[task 2017-08-12T16:28:20.883591Z] 16:28:20     INFO - GECKO(1107) |     #3 0x7fd6a5966b91  (/home/worker/workspace/build/application/firefox/libxul.so+0x21d2b91)
[task 2017-08-12T16:28:20.885543Z] 16:28:20     INFO - GECKO(1107) |     #4 0x7fd6a71712b0  (/home/worker/workspace/build/application/firefox/libxul.so+0x39dd2b0)
[task 2017-08-12T16:28:20.887451Z] 16:28:20     INFO - GECKO(1107) | -----------------------------------------------------
[task 2017-08-12T16:28:20.891087Z] 16:28:20     INFO - GECKO(1107) | Suppressions used:
[task 2017-08-12T16:28:20.895119Z] 16:28:20     INFO - GECKO(1107) |   count      bytes template
[task 2017-08-12T16:28:20.902663Z] 16:28:20     INFO - GECKO(1107) |     304       8929 libfontconfig.so
[task 2017-08-12T16:28:20.904668Z] 16:28:20     INFO - GECKO(1107) |       1         72 nss_ClearErrorStack
[task 2017-08-12T16:28:20.907612Z] 16:28:20     INFO - GECKO(1107) |      16       2316 libglib-2.0.so
[task 2017-08-12T16:28:20.910680Z] 16:28:20     INFO - GECKO(1107) | -----------------------------------------------------
[task 2017-08-12T16:28:20.915404Z] 16:28:20     INFO - GECKO(1107) | SUMMARY: AddressSanitizer: 22784 byte(s) leaked in 698 allocation(s).
[task 2017-08-12T16:28:21.949567Z] 16:28:21     INFO - GECKO(1107) | -----------------------------------------------------
[task 2017-08-12T16:28:21.951644Z] 16:28:21     INFO - GECKO(1107) | Suppressions used:
[task 2017-08-12T16:28:21.953004Z] 16:28:21     INFO - GECKO(1107) |   count      bytes template
[task 2017-08-12T16:28:21.954439Z] 16:28:21     INFO - GECKO(1107) |     694      22144 nsComponentManagerImpl
[task 2017-08-12T16:28:21.955876Z] 16:28:21     INFO - GECKO(1107) |       4        640 mozJSComponentLoader::LoadModule
[task 2017-08-12T16:28:21.957175Z] 16:28:21     INFO - GECKO(1107) |     304       8929 libfontconfig.so
[task 2017-08-12T16:28:21.958487Z] 16:28:21     INFO - GECKO(1107) |      16       2316 libglib-2.0.so
[task 2017-08-12T16:28:21.967511Z] 16:28:21     INFO - GECKO(1107) | -----------------------------------------------------
[task 2017-08-12T16:28:23.081484Z] 16:28:23     INFO - GECKO(1107) | LLVM ERROR: IO failure on output stream.
[task 2017-08-12T16:28:39.619635Z] 16:28:39     INFO - GECKO(1107) | -----------------------------------------------------
[task 2017-08-12T16:28:39.622656Z] 16:28:39     INFO - GECKO(1107) | Suppressions used:
[task 2017-08-12T16:28:39.623615Z] 16:28:39     INFO - GECKO(1107) |   count      bytes template
[task 2017-08-12T16:28:39.624569Z] 16:28:39     INFO - GECKO(1107) |     749      23776 nsComponentManagerImpl
[task 2017-08-12T16:28:39.625852Z] 16:28:39     INFO - GECKO(1107) |      49       7840 mozJSComponentLoader::LoadModule
[task 2017-08-12T16:28:39.627639Z] 16:28:39     INFO - GECKO(1107) |       1        384 pixman_implementation_lookup_composite
[task 2017-08-12T16:28:39.629348Z] 16:28:39     INFO - GECKO(1107) |     612      17514 libfontconfig.so
[task 2017-08-12T16:28:39.631177Z] 16:28:39     INFO - GECKO(1107) |       1         32 libdl.so
[task 2017-08-12T16:28:39.633702Z] 16:28:39     INFO - GECKO(1107) |      17       4348 libglib-2.0.so
[task 2017-08-12T16:28:39.635349Z] 16:28:39     INFO - GECKO(1107) |       1         40 libpulsecommon-8.0.so
[task 2017-08-12T16:28:39.637068Z] 16:28:39     INFO - GECKO(1107) | -----------------------------------------------------
[task 2017-08-12T16:28:40.336069Z] 16:28:40     INFO - TEST-INFO | Main app process: exit 0
[task 2017-08-12T16:28:40.338745Z] 16:28:40    ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | LLVMSymbolizer was unable to allocate memory.
[task 2017-08-12T16:28:40.344917Z] 16:28:40     INFO - TEST-INFO | LeakSanitizer | This will cause leaks that should be ignored to instead be reported as an error
[task 2017-08-12T16:28:40.346961Z] 16:28:40     INFO - TEST-INFO | LeakSanitizer | To show the addresses of leaked objects add report_objects=1 to LSAN_OPTIONS
[task 2017-08-12T16:28:40.348941Z] 16:28:40     INFO - TEST-INFO | LeakSanitizer | This can be done in testing/mozbase/mozrunner/mozrunner/utils.py
[task 2017-08-12T16:28:40.351495Z] 16:28:40    ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at unknown stack
Flags: needinfo?(dao+bmo)
The leaks also seem to show up on the backout (2 so far).

The bad news: The patch also triggered talos sessionstore failures in sessionrestore_many_windows https://treeherder.mozilla.org/logviewer.html#?job_id=122768357&repo=autoland
No idea what's going on. I'll break this patch up and try to land it piece by piece on try and then inbound.
Flags: needinfo?(dao+bmo)
Keywords: leave-open
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d96622625ba
Add rudimentary hover effect for urlbar icons. r=gijs
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76ba8c64110f
Move duplicate urlbar-icon rules to urlbar-searchbar.inc.css. r=gijs
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26e09dd7734b
Replace urlbar-icon margin with padding. r=gijs
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb5cc9db7dd1
Move common .urlbar-history-dropmarker styles to urlbar-searchbar.inc.css. r=gijs
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d507d6f3e0d2
Rename urlbar-icons to page-action-buttons. r=gijs
Iteration: 57.1 - Aug 15 → 57.2 - Aug 29
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d6962d0a62a
Use arrow-dropdown-16.svg for the urlbar history dropmarker. r=gijs
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee4a1ca5c948
Add urlbar-icon class to the URL bar history dropmarker. r=gijs
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/ee4a1ca5c948
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
FWIW, I have no idea what those failures were. I made no changes to the patch when splitting it up.
Something increased the fadeout gradient in the url bar in the timeframe of that last patch landing. Maybe it was that patch?

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=8febdfacc7160c9ba90ba480e8bf5cd174e4fcab&newProject=mozilla-central&newRev=4f4487cc2d30d988742109868dcf21c4113f12f5

It doesn't look like a big issue to me but I'm also not sure if it's desired to have the extra white space there.
You need to log in before you can comment on or make changes to this bug.