Closed Bug 737349 Opened 12 years ago Closed 12 years ago

Filelink account set up should contain a link to the selected provider product description

Categories

(Thunderbird :: Preferences, defect)

Product:
Thunderbird
Component:
Preferences
Version:
13 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird13 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 14.0
Tracking Flags:
Tracking Status
thunderbird13 --- fixed

People

(Reporter: jb, Assigned: mconley)

References

(Blocks 1 open bug)

Details

Attachments

(6 files, 5 obsolete files)

Updated wireframe
33.43 KB, image/png
Details
Windows 7
104.48 KB, image/png
Details
Windows XP
32.61 KB, image/png
Details
Ubuntu
78.70 KB, image/png
Details
Patch v5 (for comm-central)
13.10 KB, patch
bwinton
: review+
bwinton
: ui-review+
Details | Diff | Splinter Review
Patch v5 (for comm-aurora - carrying over r+ and ui-r+ from bwinton)
9.17 KB, patch
Bienvenu
: approval-comm-aurora+
Details | Diff | Splinter Review 
When selecting an online storage account provider, users don't necessarily know the product features. A link located next to the droplist should point to the selected provider feature page.
If there are no selected provider, this link should point to the Filelink feature description, as per 737348
Assignee: nobody → bwinton
Depends on: 737348
Assignee: bwinton → mconley
Depends on: BigFiles
Blocks: BigFiles
No longer depends on: BigFiles
Attached image Mockup for gnomestripe (obsolete) — 
From our discussion today.  Is this an acceptable target?
Comment on attachment 610136 [details]
Mockup for gnomestripe

I think I'ld prefer it if the two text links were on the same line, but I could live with this, I guess…

Thanks,
Blake.
Humm... may I ask why the learn more is "floating" on the page??? Oops.
Attached image Updated wireframe — 
The dialog is being laid out slightly different to accommodate long translation strings. See the wireframe for how we're approaching it.
Attachment #610136 - Attachment is obsolete: true
Depends on: 739279
Jb:

What URLs should we providing in these links for Dropbox and YouSendIt?

-Mike
Adding corresponding people at Dropbox & YSI for an answer. 
In the meantime, let's assume 
- http://www.dropbox.com
- http://www.yousendit.com
No longer depends on: 737348
Attached patch Patch v1 (obsolete) — — Splinter Review 
Screenshots coming next.
Hm - seems to be broken on Windows XP.  Investigating...
Attached patch Patch v2 (obsolete) — — Splinter Review 
It turns out that for Windows XP, IFrame scrollheight was being calculated differently due to those floats I was using for the "Learn more..." div.

So, instead of floating right for Dropbox and the default "Learn more...", I text-align right, and only float right for YouSendIt, who's other content ensures that the height is set correctly.
Attachment #610955 - Attachment is obsolete: true
Attachment #611025 - Flags: ui-review?(bwinton)
Attachment #611025 - Flags: review?(bwinton)
Comment on attachment 611025 [details] [diff] [review]
Patch v2

Hey Mike,

this seems to have bitrotted.  Can you update it, and maybe post some non-OSX screenshots?

Thanks,
Blake.
Attachment #611025 - Flags: ui-review?(bwinton)
Attachment #611025 - Flags: review?(bwinton)
Attached patch Patch v3 (obsolete) — — Splinter Review 
Unbitrotted - this should apply cleanly on top of the latest patch for bug 737279.
Attachment #611025 - Attachment is obsolete: true
Attachment #614153 - Flags: ui-review?(bwinton)
Attachment #614153 - Flags: review?(bwinton)
Attached image Windows 7 — 

    
    

    
  
Argh, no, doesn't apply cleanly.  Hang on...

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v4 (for comm-central) (obsolete)
        — 
          — Splinter Review
      

    


  
Ok, this patch applies cleanly on bug 739279 now.  For reals.

        Attachment #614153 -
        Attachment is obsolete: true

        Attachment #614153 -
        Flags: ui-review?(bwinton)

        Attachment #614153 -
        Flags: review?(bwinton)

        Attachment #614159 -
        Flags: ui-review?(bwinton)

        Attachment #614159 -
        Flags: review?(bwinton)

    
  

        Attachment #614159 -
        Attachment description: Patch v4 → Patch v4 (for comm-central)

    
    

    
  

      
      
      
      

        Attached image
          
        Windows XP
        — 
      

    


  

    
    

    
  
Comment on attachment 614159 [details] [diff] [review]
Patch v4 (for comm-central)

Looks like the merge didn't go so well.

I'm going to stop rushing this thing.  I'll have an updated, correct patch tomorrow.

        Attachment #614159 -
        Flags: ui-review?(bwinton)

        Attachment #614159 -
        Flags: review?(bwinton)

    
    

    
  

      
      
      
      

        Attached image
          
        Ubuntu
        — 
      

    


  

    
    

    
  


  
Alright, this patch does the job.  Seriously this time.

        Attachment #614159 -
        Attachment is obsolete: true

        Attachment #614468 -
        Flags: ui-review?(bwinton)

        Attachment #614468 -
        Flags: review?(bwinton)

    
    

    
  
Comment on attachment 614468 [details] [diff] [review]
Patch v5 (for comm-central)

>+++ b/mail/components/cloudfile/content/Dropbox/settings.xhtml
>@@ -1,17 +1,21 @@
>+<!DOCTYPE html [
>+  <!ENTITY % htmlDTD PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "DTD/xhtml1-strict.dtd"> %htmlDTD;
>+  <!ENTITY % dbDTD SYSTEM "chrome://messenger/locale/cloudfile/Dropbox/settings.dtd"> %dbDTD;
>+]>
> <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">

So, we need some direction stuff on this to correctly handle rtl languages, but if you wanted to do that as a separate bug, I could accept that.

Other than that, I like it.

r=me, ui-r=me!

Later,
Blake.

        Attachment #614468 -
        Flags: ui-review?(bwinton)

        Attachment #614468 -
        Flags: ui-review+

        Attachment #614468 -
        Flags: review?(bwinton)

        Attachment #614468 -
        Flags: review+

    
  
Blocks: 744904

    
    

    
  


  
This patch is mostly the same as the patch for comm-central (attachment 614468 [details] [diff] [review]), except for no new strings - we temporarily borrow some from messenger.properties.

The biggest difference, besides removing the string insertions, is the function "insertLearnMoreString" in addAccountDialog.js, and the points where that function is called.

I'll go ahead and request comm-aurora approval, but if someone feels I should get this reviewed first, let me know.

        Attachment #614536 -
        Flags: approval-comm-aurora?

    
    

    
  
Patch for comm-central (Attachment 614468 [details] [diff]) landed in comm-central as http://hg.mozilla.org/comm-central/rev/f2d47bbb6bf3
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0

    
  

        Attachment #614536 -
        Flags: approval-comm-aurora? → approval-comm-aurora+

    
    

    
  
Comment on attachment 614468 [details] [diff] [review]
Patch v5 (for comm-central)

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

::: mail/components/cloudfile/content/Dropbox/settings.xhtml
@@ +10,1 @@
>  <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">

This will be incorrect as we localize it.

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: