<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Daksh,<div class=""><br class=""></div><div class="">Thanks for the updated patch, looks good. I just pushed it to the master branch.</div><div class=""><br class=""></div><div class="">BR. David<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">Am 10.05.2018 um 00:05 schrieb Daksh Shah <<a href="mailto:daksh17336@iiitd.ac.in" class="">daksh17336@iiitd.ac.in</a>>:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Updated patch attached. Replies inline :)<br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, May 9, 2018 at 3:02 AM David Fuhrmann <<a href="mailto:david.fuhrmann@gmail.com" class="">david.fuhrmann@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class="">Hello Daksh,<div class=""><br class=""></div><div class="">Maybe your misunderstood my question a bit. I was asking if you should set the same value for both NSAccessibilityDescriptionAttribute and NSAccessibilityTitleAttribute, or whether setting only one attribute would be enough as well. But if you do now know, then we can also leave it as is.</div></div></blockquote><div class="">About that, I had a look at <a href="https://stackoverflow.com/q/8423465/2806163" class="">https://stackoverflow.com/q/8423465/2806163</a> and it seems like it is exactly what you described. The Description used there is related to VoiceOver. I tried using it myself but I could not get VoiceOver to work on the buttons in Full Screen. Maybe there is a bug or I was doing it wrong, but that is what it is intended to do, as far as my understanding is. </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">Also see inline.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div class="">From 0716d8e311f4199fbb45fef5205bbe76ede4af22 Mon Sep 17 00:00:00 2001</div><div class="">From: Daksh Shah <<a href="mailto:daksh17336@iiitd.ac.in" target="_blank" class="">daksh17336@iiitd.ac.in</a>></div><div class="">Date: Sun, 6 May 2018 19:13:41 +0530</div><div class="">Subject: [PATCH] macosx: Make toolTips on Full Screen consistent</div><div class=""><br class=""></div><div class="">Fix #19993. Now the tooltips which are shown in the Normal View are same</div></blockquote></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class="">as the ones which are shown in Full-Screen View. Adapted the shorter version.</div></blockquote></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class="">And also, changed the Normal View toolTip of "Fullscreen" to "Enter fullscreen“</div></blockquote><div class=""><br class=""></div><div class="">You should write „fixes #19993“ as a separate and last paragraph of you commit message.</div></div></div></blockquote><div class="">Done </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">---</div><div class=""> modules/gui/macosx/VLCControlsBarCommon.m | 2 +-</div><div class=""> modules/gui/macosx/VLCFSPanelController.m | 13 ++++++++-----</div><div class=""> 2 files changed, 9 insertions(+), 6 deletions(-)</div><div class=""><br class=""></div><div class="">diff --git a/modules/gui/macosx/VLCControlsBarCommon.m b/modules/gui/macosx/VLCControlsBarCommon.m</div><div class="">index 996b9aa92f..93de7151d0 100644</div><div class="">--- a/modules/gui/macosx/VLCControlsBarCommon.m</div><div class="">+++ b/modules/gui/macosx/VLCControlsBarCommon.m</div><div class="">@@ -78,7 +78,7 @@ - (void)awakeFromNib</div><div class=""> if (_darkInterface)</div><div class=""> [self.timeSlider setSliderStyleDark];</div><div class=""> </div><div class="">- [self.fullscreenButton setToolTip: _NS("Fullscreen")];</div><div class="">+ [self.fullscreenButton setToolTip: _NS("Enter fullscreen")];</div><div class=""> [[self.fullscreenButton cell] accessibilitySetOverrideValue:[self.fullscreenButton toolTip] forAttribute:NSAccessibilityDescriptionAttribute];</div><div class=""> </div><div class=""> if (!_darkInterface) {</div><div class="">diff --git a/modules/gui/macosx/VLCFSPanelController.m b/modules/gui/macosx/VLCFSPanelController.m</div><div class="">index b16f47fafe..3a83665239 100644</div><div class="">--- a/modules/gui/macosx/VLCFSPanelController.m</div><div class="">+++ b/modules/gui/macosx/VLCFSPanelController.m</div><div class="">@@ -92,7 +92,7 @@ - (void)windowDidLoad</div><div class=""> <span class="Apple-converted-space"> </span>forAttribute:NSAccessibilityTitleAttribute]; \</div><div class=""> [target accessibilitySetOverrideValue:desc \</div><div class=""> <span class="Apple-converted-space"> </span>forAttribute:NSAccessibilityDescriptionAttribute]; \</div><div class="">- [target setToolTip:desc];</div><div class="">+ [target setToolTip:title];</div></blockquote><div class=""><br class=""></div><div class="">Ok, this looks fine.</div><br class=""><blockquote type="cite" class=""><div class=""> </div><div class=""> - (void)setupControls</div><div class=""> {</div><div class="">@@ -107,17 +107,17 @@ - (void)setupControls</div><div class=""> _NS("Previous"),</div></blockquote></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""> _NS("Go to the previous item"));</div></blockquote></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""> setupButton(_forwardButton,</div><div class="">- _NS("Forward"),</div><div class="">+ _NS("Seek Forward"),</div><div class=""> _NS("Seek forward"));</div></blockquote><div class=""><br class=""></div><div class="">This does not unify things, but actually makes the text different between fullscreen controller and controls bar now.</div><div class="">In controls bar, we already use „Forward“ and "Seek forward“.</div><div class=""><br class=""></div><div class="">I would probably keep it as is for now.</div></div></div></blockquote><div class="">The reason I changed it was, it would clear the slight ambiguity which comes owing to the Next and Previous buttons which show up only in the Full screen by default.</div><div class="">But maybe it is better to stick with the old Heading. Updated in the commit </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""> setupButton(_backwardButton,</div><div class="">- _NS("Backward"),</div><div class="">+ _NS("Seek Backward"),</div><div class=""> _NS("Seek backward"));</div></blockquote><div class=""><br class=""></div><div class="">Same as above.</div></div></div></blockquote><div class="">Updated </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><br class=""></div><div class="">Also, having both „Seek Backward“, and „Seek backward“ (with lower case b) does not make much sense.</div></div></div></blockquote><div class="">The second one would have been in VoiceOver, so I thought it would not make any difference. But changing the title to original as you suggested </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""> setupButton(_fullscreenButton,</div><div class="">- _NS("Toggle Fullscreen mode"),</div><div class="">+ _NS("Leave Fullscreen"),</div><div class=""> _NS("Leave fullscreen mode"));</div></blockquote><div class=""><br class=""></div><div class="">The change itself seems to be ok. But why should we have both „Leave fullscreen“, and then „Leave fullscreen mode“ below? Both sound too similar for me.</div></div></div></blockquote><div class="">Just that the description is to be read out loud by the computer, so it would blend in more, was my reasoning to having kept it as is </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""> setupButton(_volumeSlider,</div><div class=""> _NS("Volume"),</div><div class="">- _NS("Adjust the volume"));</div><div class="">+ _NS("Adjust the Volume"));</div></blockquote><div class=""><br class=""></div>Why? Volume should stay lower case here I think.</div></div></blockquote><div class="">Updated </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""> setupButton(_timeSlider,</div><div class=""> _NS("Position"),</div></blockquote></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""> _NS("Adjust the current playback position"));</div></blockquote></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class="">@@ -221,11 +221,13 @@ - (IBAction)volumeSliderUpdate:(id)sender</div><div class=""> - (void)setPlay</div><div class=""> {</div><div class=""> [_playPauseButton setState:NSOffState];</div><div class="">+ [_playPauseButton setToolTip: _NS("Play")];</div><div class=""> }</div></blockquote><div class=""><br class=""></div>Ok.</div><div class=""><br class=""><blockquote type="cite" class=""><div class=""> </div><div class=""> - (void)setPause</div><div class=""> {</div><div class=""> [_playPauseButton setState:NSOnState];</div><div class="">+ [_playPauseButton setToolTip: _NS("Pause")];</div><div class=""> }</div></blockquote><div class=""><br class=""></div>Ok.<br class=""><blockquote type="cite" class=""><div class=""> </div><div class=""> - (void)setStreamTitle:(NSString *)title</div><div class="">@@ -292,6 +294,7 @@ - (void)setSeekable:(BOOL)seekable</div><div class=""> - (void)setVolumeLevel:(int)value</div><div class=""> {</div><div class=""> [_volumeSlider setIntValue:value];</div><div class="">+ [_volumeSlider setToolTip: [NSString stringWithFormat:_NS("Volume: %i %%"), (value*200)/AOUT_VOLUME_MAX]];</div><div class=""> }</div><div class=""> </div></blockquote><div class=""><br class=""></div>Ok.</div><div class=""><br class=""><blockquote type="cite" class=""><div class=""> #pragma mark -</div><div class="">-- </div><div class="">2.15.1 (Apple Git-101)</div><div class=""><br class=""></div></blockquote><br class=""></div><div class="">Best regards,</div><div class="">David</div><div class=""><br class=""></div><div class=""><div class=""><br class=""><blockquote type="cite" class=""></blockquote></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><blockquote type="cite" class=""><div class="">Am 06.05.2018 um 16:27 schrieb Daksh Shah <<a href="mailto:daksh17336@iiitd.ac.in" target="_blank" class="">daksh17336@iiitd.ac.in</a>>:</div><br class="m_4262495554261952669Apple-interchange-newline"></blockquote></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">Hi, <div class=""><br class=""></div><div class="">I have updated the patch, kindly have a look. Yes, you were right about it not making a lot of sense to set the description same as I found out that in the normal view, the tooltip was being set to title(NSAccessibilityTitleAttribute). But in FullScreen, it was being set to description. So I changed the setupButton function and now the tooltip is being set to the title rather than the description (For FS View as well)</div><div class=""><br class="">I also updated some of the values for that. Let me know if any changes are needed :)<br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Mar 21, 2018 at 11:23 PM David Fuhrmann <<a href="mailto:david.fuhrmann@gmail.com" target="_blank" class="">david.fuhrmann@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class="">HI Daksh,<div class=""><br class=""></div><div class="">See below.<br class=""><div class=""></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">Am 21.03.2018 um 12:00 schrieb Daksh Shah <<a href="mailto:daksh17336@iiitd.ac.in" target="_blank" class="">daksh17336@iiitd.ac.in</a>>:</div><br class="m_4262495554261952669m_-813062079578191380Apple-interchange-newline"><div class=""><div dir="ltr" class="">Thanks for your feedback. Kindly have a look at the replies in-line<br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Mar 21, 2018 at 4:28 AM David Fuhrmann <<a href="mailto:david.fuhrmann@gmail.com" target="_blank" class="">david.fuhrmann@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Hello,<br class=""><br class="">- _NS("Go to next item"));<br class="">+ _NS("Forward"));<br class=""><br class="">This looks very misleading. This button is about jumping to next item, not about forwarding the same item.</blockquote><div class="">I am sorry. I will fix that and change the toolTip to "Next" </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><br class="">Just below, there is another button for "Seek forward“. So you should not choose a description which is too close to the other button (as its done in your proposal).<br class=""></blockquote><div class="">By proposal you mean this patch right? </div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><br class="">If you compare to the buttons descriptions in the main window: Please note that those buttons have different features, depending if only a back and forward button is shown, or if „next“ / „prev“ buttons are shown additionally.<br class=""></blockquote><div class="">I understood that now. I think that it is another place where some enhancement can be done. When the two new buttons show up, the toolTips remain the same. And the functionalities change. This does not seem intuitive for new users. <br class="">The only reason I realized that was because you pointed it out.</div><div class=""><br class=""></div><div class="">Should I try to change this?</div></div></div></div></blockquote><div class=""><br class=""></div></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><div class="">Sure, if you want to.</div></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><br class=""><br class="">- _NS("Go to the previous item"));<br class="">+ _NS("Backward"));<br class=""><br class="">Same here.<br class=""><br class=""> setupButton(_timeSlider,<br class=""> _NS("Position"),<br class="">- _NS("Adjust the current playback position"));<br class="">+ _NS("Position"));<br class=""><br class="">Are you sure choosing the same string for Accessibility title and accessibility description is the correct thing to do? I would rather assume one might leave the description blank if you actually have no description (but I’m just guessing here…)<br class=""></blockquote><div class="">I could not understand properly. The title is what shows up in the toolTip. But is there a place where the description shows up? I kept it just in case it might be describing the button in some other place.</div><div class="">If not, I think we should remove it. </div></div></div></div></blockquote><div class=""><br class=""></div></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class="">Your patch modifies the attribute NSAccessibilityDescriptionAttribute, mainly. Would be great if you can check what this is and where its used in detail, and if you can find any guidelines how it should be filled. (IIRC is has something to do if you use accessibility options like voice control or similar, but I do not remember details out of my head.)</div><div class=""><br class=""></div><div class="">Just from the naming of both attributes, I find it weird to have a „title“ and a „description“ with the same content.</div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(120, 73, 42);" class=""><br class=""></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><br class="">BR. David<br class=""><br class="">> Am 19.03.2018 um 12:02 schrieb Daksh Shah <<a href="mailto:daksh17336@iiitd.ac.in" target="_blank" class="">daksh17336@iiitd.ac.in</a>>:<br class="">><br class="">> Now the tooltips which are shown in the Normal View are same as the ones which are shown in Full-Screen View. Adapted the shorter version.<br class="">> And also, changed the Normal View toolTip of "Fullscreen" to "Enter fullscreen"<br class="">><br class="">> <0001-Fixed-19993-Made-toolTips-on-Full-Screen-consistent.patch>_______________________________________________<br class="">> vlc-devel mailing list<br class="">> To unsubscribe or modify your subscription options:<br class="">><span class="Apple-converted-space"> </span><a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank" class="">https://mailman.videolan.org/listinfo/vlc-devel</a><br class=""><br class="">_______________________________________________<br class="">vlc-devel mailing list<br class="">To unsubscribe or modify your subscription options:<br class=""><a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank" class="">https://mailman.videolan.org/listinfo/vlc-devel</a></blockquote></div></div>_______________________________________________<br class="">vlc-devel mailing list<br class="">To unsubscribe or modify your subscription options:<br class=""><a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank" class="">https://mailman.videolan.org/listinfo/vlc-devel</a></div></blockquote></div><br class=""></div></div>_______________________________________________<br class="">vlc-devel mailing list<br class="">To unsubscribe or modify your subscription options:<br class=""><a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank" class="">https://mailman.videolan.org/listinfo/vlc-devel</a></blockquote></div></div></div></div></blockquote></div></div></div><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class=""><div class=""><blockquote type="cite" class=""><div class=""><span id="m_4262495554261952669cid:16335d4a1623f7bf3031" class=""><0001-macosx-Make-toolTips-on-Full-Screen-consistent.patch></span></div></blockquote></div><br class=""></div></div></blockquote></div></div><span id="cid:16346f033143f7bf3031"><0001-macosx-Make-toolTips-on-Full-Screen-consistent.patch></span></div></blockquote></div><br class=""></div></body></html>