magicp Reporter
	Description • 8 years ago

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160307063917

Steps to reproduce:

1. Start Nightly
2. Go to "about:home"
3. Open DevTools > Memory
4. Check-on "Record allocation stacks"
5. Reload current page
6. Switch group-by from "Type" to "Call Stack"
7. Take snapshot
8. Select any rows from heap-view


Actual results:

frame-links are not enabled focused color. This bug was fixed once by Bug 1242646.

Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=5cb797a911407cbddecd4f0c347ca0da251cff7a&tochange=9b3c9c05e11c7406969b9f4246f83a0422e084ed


Expected results:

Focused color should be enabled.

components-frame.css - line:15, should move to bottom.

.focused .frame-link-filename,
.focused .frame-link-column,
.focused .frame-link-line,
.focused .frame-link-host,
.focused .frame-link-colon {
  color: var(--theme-selection-color);
}

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]
	Comment 1 • 8 years ago

Thanks for the report and regression range to boot!

	Jordan Santell [:jsantell] [@jsantell] Assignee
	Comment 2 • 8 years ago

Thanks for the background info -- I'll grab this and get it uplifted to Dev Edition

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]
	Comment 4 • 8 years ago

Comment on attachment 8728693 [details] [diff] [review]
1254444-focus-frame.patch

Review of attachment 8728693 [details] [diff] [review]:
-----------------------------------------------------------------

r=me assuming you tested this locally and verified the selected row is legible

	Jordan Santell [:jsantell] [@jsantell] Assignee
	Comment 5 • 8 years ago

Separate patch for Aurora for clean landing:

Approval Request Comment
[Feature/regressing bug #]: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=5cb797a911407cbddecd4f0c347ca0da251cff7a&tochange=9b3c9c05e11c7406969b9f4246f83a0422e084ed
[User impact if declined]: Difficult to read source names in the memory tool
[Describe test coverage new/current, TreeHerder]: CSS change
[Risks and why]: If we don't land, users will have difficulty reading source names in the memory tool, and looks rather unpolished.

	Jordan Santell [:jsantell] [@jsantell] Assignee
	Comment 6 • 8 years ago

(In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from comment #4)
> Comment on attachment 8728693 [details] [diff] [review]
> 1254444-focus-frame.patch
> 
> Review of attachment 8728693 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me assuming you tested this locally and verified the selected row is
> legible

Good thing too, because the previous patch just landed caused the anchor selector to take precedent over the focused classes

	Pulsebot
	Comment 7 • 8 years ago

https://hg.mozilla.org/integration/fx-team/rev/f7abc73ebddb

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 8 • 8 years ago

(In reply to Jordan Santell [:jsantell] [@jsantell] (Please needinfo) from comment #6)
> (In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from comment #4)
> > Comment on attachment 8728693 [details] [diff] [review]
> > 1254444-focus-frame.patch
> > 
> > Review of attachment 8728693 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > r=me assuming you tested this locally and verified the selected row is
> > legible
> 
> Good thing too, because the previous patch just landed caused the anchor
> selector to take precedent over the focused classes

Jordan, does this mean that this fix needs more work? I also noticed the "leave-open" keyword. Will there be a part 2? If yes, I'd much rather uplift "the good final" fix in one go. Thanks!

	Jordan Santell [:jsantell] [@jsantell] Assignee
	Comment 9 • 8 years ago

The leave-open was just for keeping this open until the aurora patch landed - sorry for the confusion!

	Carsten Book [:Tomcat]
	Comment 10 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f7abc73ebddb

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 11 • 8 years ago

magicp, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 12 • 8 years ago

Comment on attachment 8728702 [details] [diff] [review]
1254444-aurora.patch

CSS change, regression fix, taking it in Aurora47.

	magicp Reporter
	Comment 13 • 8 years ago

(In reply to Ritu Kothari (:ritu) from comment #11)
> magicp, could you please verify this issue is fixed as expected on a latest
> Nightly build? Thanks!

This issue was fixed on Nightly. Thanks.

	Carsten Book [:Tomcat]
	Comment 14 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/13f341508044

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 15 • 8 years ago

(In reply to magicp from comment #13)
> Created attachment 8730382 [details]
> Bug-1254444-verified-48.0a1.png
> 
> (In reply to Ritu Kothari (:ritu) from comment #11)
> > magicp, could you please verify this issue is fixed as expected on a latest
> > Nightly build? Thanks!
> 
> This issue was fixed on Nightly. Thanks.

Great!

	Forhad Hossain
	Comment 16 • 8 years ago

I have reproduced this bug with Nightly 48.0a1 (2016-03-08)  with instruction from comment 0 and Linux 32 Bit.     

Verified as fixed with Latest Developer Edition 47.0a2 (2016-04-16) (Build ID:20160416004025)

User Agent : Mozilla/5.0 (X11; Linux i686; rv:47.0) Gecko/20100101 Firefox/47.0

[testday-20160415]