Fixed #5576 = Trash icon is missing from whitelist cell

Fixed #5576 = Trash icon is missing from whitelist cell

[dean, 2017-08-25 12:00:23]

On 2017/08/25 10:52:45, a.shephard wrote:

LGTM - Renders correctly from my tests.

[d108, 2017-08-28 07:58:35]

I've confirmed the fix of the trash icon, good one, but there is a patch issue.

Here is my procedure:

Clone with

$ git clone https://github.com/adblockplus/adblockplussafariios.git && pod
install --project-directory=adblockplussafariios && cd adblockplussafariios

Download patch, convert with patchconv, and apply with

$ git apply --reject --whitespace=fix ../../../git.patch

Result is

Checking patch AdblockPlusSafari/Base.lproj/Main.storyboard...
error: while searching for:
                                </constraints>
                            </imageView>
                            <label opaque="NO" userInteractionEnabled="NO"
contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251"
text="To get a better ad blocking experience, use Adblock *Browser* for iOS"
textAlignment="center" lineBreakMode="wordWrap" numberOfLines="3"
baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO"
translatesAutoresizingMaskIntoConstraints="NO" id="VJZ-V3-ohd">
                                <rect key="frame" x="15" y="539" width="345"
height="43"/>
                                <fontDescription key="fontDescription"
name="SourceSansPro-Light" family="Source Sans Pro" pointSize="17"/>
                                <color key="textColor" red="0.0" green="0.0"
blue="0.0" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
                                <nil key="highlightedColor"/>

error: patch failed: AdblockPlusSafari/Base.lproj/Main.storyboard:803
Applying patch AdblockPlusSafari/Base.lproj/Main.storyboard with 1 reject...
Rejected hunk #1.
Hunk #2 applied cleanly.

**The first patch hunk did not apply successfully** This could be due to an
Xcode update - see below. However, it's a mystery to me as I applied the patch
before letting Xcode touch anything.

---

I noticed an unrelated label ht change that I believe is due to an Xcode update.

From

<rect key="frame" x="15" y="539" width="345" height="43"/>

To

<rect key="frame" x="15" y="539" width="345" height="40"/>

**I know opening the storyboard in different Xcodes can make such changes
automatically but I think we need to be careful about committing such changes if
we are not intending them.**

For the record, my height for that label ends up being 40 without the patch so
it's likely due to an Xcode change.

---

I'm not sure why the first patch hunk fails. For reference, my Xcode text
editing settings are here
(https://drive.google.com/open?id=0B3HwydeogdcKUjMtek14TFlueHc). It could be a
mismatch in line endings or other problem on my end. We may need to sync up our
text editing settings.

[a.shephard, 2017-08-28 08:54:28]

On 2017/08/28 07:58:35, Daniel wrote:
> I've confirmed the fix of the trash icon, good one, but there is a patch
issue.
> 
> Here is my procedure:
> 
> Clone with
> 
> $ git clone https://github.com/adblockplus/adblockplussafariios.git && pod
> install --project-directory=adblockplussafariios && cd adblockplussafariios
> 
> Download patch, convert with patchconv, and apply with
> 
> $ git apply --reject --whitespace=fix ../../../git.patch
> 
> Result is
> 
> Checking patch AdblockPlusSafari/Base.lproj/Main.storyboard...
> error: while searching for:
>                                 </constraints>
>                             </imageView>
>                             <label opaque="NO" userInteractionEnabled="NO"
> contentMode="left" horizontalHuggingPriority="251"
verticalHuggingPriority="251"
> text="To get a better ad blocking experience, use Adblock *Browser* for iOS"
> textAlignment="center" lineBreakMode="wordWrap" numberOfLines="3"
> baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO"
> translatesAutoresizingMaskIntoConstraints="NO" id="VJZ-V3-ohd">
>                                 <rect key="frame" x="15" y="539" width="345"
> height="43"/>
>                                 <fontDescription key="fontDescription"
> name="SourceSansPro-Light" family="Source Sans Pro" pointSize="17"/>
>                                 <color key="textColor" red="0.0" green="0.0"
> blue="0.0" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
>                                 <nil key="highlightedColor"/>
> 
> error: patch failed: AdblockPlusSafari/Base.lproj/Main.storyboard:803
> Applying patch AdblockPlusSafari/Base.lproj/Main.storyboard with 1 reject...
> Rejected hunk #1.
> Hunk #2 applied cleanly.
> 
> **The first patch hunk did not apply successfully** This could be due to an
> Xcode update - see below. However, it's a mystery to me as I applied the patch
> before letting Xcode touch anything.
> 
> ---
> 
> I noticed an unrelated label ht change that I believe is due to an Xcode
update.
> 
> From
> 
> <rect key="frame" x="15" y="539" width="345" height="43"/>
> 
> To
> 
> <rect key="frame" x="15" y="539" width="345" height="40"/>
> 
> **I know opening the storyboard in different Xcodes can make such changes
> automatically but I think we need to be careful about committing such changes
if
> we are not intending them.**
> 
> For the record, my height for that label ends up being 40 without the patch so
> it's likely due to an Xcode change.
> 
> ---
> 
> I'm not sure why the first patch hunk fails. For reference, my Xcode text
> editing settings are here
> (https://drive.google.com/open?id=0B3HwydeogdcKUjMtek14TFlueHc). It could be a
> mismatch in line endings or other problem on my end. We may need to sync up
our
> text editing settings.

Just created and applied a patch locally and had no issues.
Also noticed the code change in regards to the rect height change. Not sure what
has caused this exactly, but most likely related to the magic of Xcode.
Will send you the created patch set (locally-created) for you to verify.

[Felix Dahlke, 2017-08-28 16:18:49]

LGTM