Closed Bug 1395174 Opened 3 years ago Closed 3 years ago

iOS 11: Settings screens such as New Tab, Open Links in, don't show default selection


(Firefox for iOS :: General, defect, P2)




Tracking Status
fxios 9.0 ---
fxios-v9.0 --- affected
fxios-v10.0 --- affected


(Reporter: csuciu, Assigned: garvan)



(Whiteboard: [MobileCore])


(1 file, 1 obsolete file)

55 bytes, text/x-github-pull-request
: review+
Details | Review
iPad Mini (iOS 11b6)
v9.0 (5607)

* Without having a homepage set, go to Settings => New Tab

Expected: "Show your Homepage" option should be grayed out.

Actual: "Show your Homepage" option appears to be available. Nothing happens when tapping on it.

After selecting one other option from the list (e.g Show your Bookmarks), the option turns grayed out.
Blocks: ios11
Same issue in the "Open mail links in" screen.

All options appear to be available even though not all supported mail clients are installed on the device.
Priority: -- → P2
looks like a regression from your settings changes.
Flags: needinfo?(gkeeley)
i'll take a look
Assignee: nobody → gkeeley
Flags: needinfo?(gkeeley)
Attached file Pull request (obsolete) —
Attachment #8903631 - Flags: review?(jhugman)
This settings screen is one of the settings views that is not a SettingsTableViewController and doesn't share settings UI code, but i reverted all my commits on the project to ensure my settings changes had no effect
Please update bug titles, I just noticed Comment #1. This is clearly an iOS 11 tableview bug. 
That is: viewWillAppear() tableview reload isn't working, but viewDidAppear() does.
Summary: "New Tab setting: Show your homepage" appears to be available when there's no homepage set → iOS 11: Settings screens such as New Tab, Open Links in, don't show default selection
Comment on attachment 8903631 [details] [review]
Pull request

This is a widespread bug, I'll get this isolated and filed to Apple and OpenRadar. 

We should wait for the GM and recheck this.
Attachment #8903631 - Flags: review?(jhugman) → review-
On iOS 11
  cell.textLabel?.textColor = .pink;
Doesn't work if done before viewDidAppear(). (textLabel is non-nil, and has the correct string set)

This does work if we want to do this for enabled/disabled states instead:
   cell.contentView.alpha = enabled ? 1.0 : 0.3

My first attempt at a test case didn't repro the bug, trying again to make a standalone case, now that I see exactly what does and doesn't work.
Attached file Pull request
Figured this out. My comments are on the commit:

"iOS 11 table cell text color can conflict with NSAttributedString color

In cellForRowAt we are setting the cell.textLabel.color and the color attribute on NSAttributedString in some cases, and because of enabled/disabled styling we are setting them differently. 
On iOS 11 we see that the latter is trumping during first creation, and then on cell reuse the former is applied.

The fix is easy. But to protect against future misuse, I am changing our utility function for getting
table row text to force the developer to decide if you want the enabled or disabled color.
If they then go and combine this with cell.textLabel.color, well, good luck to them."
Attachment #8903631 - Attachment is obsolete: true
Attachment #8903785 - Flags: review?(jhugman)
Attachment #8903785 - Flags: review?(jhugman) → review+
Landed on master and v9.x
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: [MobileCore]
You need to log in before you can comment on or make changes to this bug.